[PNP-9586] Add manual title concern #5027
Conversation
sairamya93
force-pushed
the
model-manual-section
branch
from
201f150 to
c27e2fb
Compare
sairamya93
changed the title
sairamya93
force-pushed
the
model-manual-section
branch
from
c27e2fb to
7d37e9a
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
345ff38 to
9a806db
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
9a806db to
b376cd9
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
b376cd9 to
a96fefb
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
a96fefb to
40f5508
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
40f5508 to
42d67ac
Compare
app/models/concerns/manual_parts.rb
Outdated
|
|
||
| included do | ||
| def title | ||
| content_store_response["links"]["manual"].first["title"] |
There was a problem hiding this comment.
This could be linked("manual").first.title
| end | ||
|
|
||
| def hmrc? | ||
| %w[hmrc_manual hmrc_manual_section].include?(schema_name) |
There was a problem hiding this comment.
Is this manual parts concern going to be included in hmrc_manual items, or just hmrc_manual_section items?
There was a problem hiding this comment.
I suppose we would have to refactor manual content_item too to include the relevant methods required in one concern. We aim to to avoid dependency of methods between concerns. In that case hmrc_manual would need to include the shared manual_parts concern.
app/models/concerns/manual_parts.rb
Outdated
| end | ||
|
|
||
| def manual_content_item | ||
| @manual_content_item ||= Services.content_store.content_item(base_path) |
There was a problem hiding this comment.
I think we shouldn't have this - is this supposed to be pointing to the manual that this manual section is linked to? If so, we should be handling it via linked("manual") - or some function built into that if the linked item doesn't have enough information.
There was a problem hiding this comment.
We actually use the method parsed_content_item here. I am not sure if we really require here as we have not migrated it yet but looks like we can possibly avoid it.
| title = content_store_response["title"] || "" | ||
| title += " - " if title.present? | ||
|
|
||
| if hmrc? |
There was a problem hiding this comment.
Do we need to worry about this in the shared concern? It might make more sense to have this in the individual models or in the view and the if/else statement wouldn't be needed then.
There was a problem hiding this comment.
Hmm the particular concern - manual_parts consists of page_title which makes a call to manual_page_title alias method. As that method is in manual content_item in government_frontend,I have added it here along with the dependent method i.e hmrc?
If I remember correctly, we had decided on not having methods in one concern calling methods in another concern.
As both hmrc_manual and hmrc_manual_section call this method through manual content_item, I have included that in this shared concern.
sairamya93
force-pushed
the
model-manual-section
branch
from
42d67ac to
47a5f0f
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
47a5f0f to
55dd662
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
55dd662 to
7a53eeb
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
7a53eeb to
adfd722
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
adfd722 to
2e92287
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
2e92287 to
a8fa346
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
a8fa346 to
3f813ad
Compare
sairamya93
force-pushed
the
model-manual-section
branch
from
3f813ad to
b4e5ed8
Compare
- Added manual_sections concern from government-frontend for the routes to be migrated: manual_section and hmrc_manual_section to frontend in future PRs. Audit trail: https://github.com/alphagov/government-frontend/blob/6d6710d9fe6439e9e949e3d9a832facdfc73e71e/app/presenters/content_item/manual_section.rb - Added title and page_title methods along with the related tests. manual_page_title is included in this concern itself instead of having it as an alias in manual content_item in government_frontend Audit trail for the tests: https://github.com/alphagov/government-frontend/blob/c035e4ba316b90a485b4cf34cacb8f6fa4a64c00/test/presenters/content_item/manual_section_test.rb#L19 Ref.: https://github.com/alphagov/government-frontend/blob/6d6710d9fe6439e9e949e3d9a832facdfc73e71e/app/presenters/content_item/manual.rb#L16 - The other methods in the content_item are added in the subsequent commits
- Copied over the translation keys for manuals from government-frontend Ran the command: rake "consolidation:copy_translation[manuals,formats.manuals] Audit trail: https://github.com/alphagov/government-frontend/blob/main/config/locales/en.yml#L352
- Added the method that will provide the document heading for manual section layout here https://github.com/alphagov/government-frontend/blob/3012ee93eeba7f9fbc58dba09a0f9e04d5691cf6/app/views/content_items/manuals/_manual_section_layout.html.erb#L16 Audit trail: https://github.com/alphagov/government-frontend/blob/c035e4ba316b90a485b4cf34cacb8f6fa4a64c00/app/presenters/content_item/manual_section.rb#L11 Audit trail for test: https://github.com/alphagov/government-frontend/blob/c035e4ba316b90a485b4cf34cacb8f6fa4a64c00/test/presenters/content_item/manual_section_test.rb#L31
- Added in the related methods - show_contents_list? which has a defined content_id for MOJ. Breadcrumbs method provides the title and url. Ref. https://github.com/alphagov/government-frontend/blob/3012ee93eeba7f9fbc58dba09a0f9e04d5691cf6/app/presenters/manual_section_presenter.rb#L61
- Manual base path is retrieved from the presenters in government-frontend for hmrc_manual_section_presenter and manual_section_presenter.We retrieve manual base path from the details in content_item as they have the same base path. Audit trail for hmrc_manual_section_presenter: https://github.com/alphagov/government-frontend/blob/7fce66e581e7ddf3ea3e3f2f7efcf8db45a9f0f6/app/presenters/hmrc_manual_section_presenter.rb#L6 Audit trail for manual_section_presenter: https://github.com/alphagov/government-frontend/blob/7fce66e581e7ddf3ea3e3f2f7efcf8db45a9f0f6/app/presenters/manual_section_presenter.rb#L9
- Copied over the method from government-frontend and modified the memoizing functionality to use the frontend's ContentItemLoader instead of Services. Also added in the related test for it. Audit trail: https://github.com/alphagov/government-frontend/blob/7fce66e581e7ddf3ea3e3f2f7efcf8db45a9f0f6/app/presenters/content_item/manual_section.rb#L40 Audit trail for test: https://github.com/alphagov/government-frontend/blob/7fce66e581e7ddf3ea3e3f2f7efcf8db45a9f0f6/test/presenters/content_item/manual_section_test.rb#L79
sairamya93
force-pushed
the
model-manual-section
branch
from
b4e5ed8 to
706e597
Compare
| expect(described_class.new(content_item).page_title).to eq("Content design: planning, writing and managing content - What is content design? - Guidance") | ||
| end | ||
|
|
||
| it "does not add separator when title is blank" do |
There was a problem hiding this comment.
Should this test also be included under the context "when schema is hmrc manual"?
There was a problem hiding this comment.
Separate clarification Q - when we say separator are we referring to the hyphen? If so, should the title be empty on L26:
expect(described_class.new(content_item).page_title).to eq("Content design: planning, writing and managing content - Guidance")
| end | ||
| end | ||
|
|
||
| RSpec.shared_examples "it can have breadcrumbs" do |document_type, example_name| |
There was a problem hiding this comment.
Can we include a link to the tests for the audit trail in the commit description?
4 participants
What
Move
manual_sectioncontent_item fromgovernment_frontendtofrontend.Note:
publishedandother_metadatamethods in the content_item would be handled in the corresponding shared manual header views as they contain thepublic_updated_datemetadata.Why
manual_sectioncontent_item is shared withmanual_sectionandhmrc_manual_sectiondocument_types. We migrate the shared component first to avoid conflicts and duplication when moving over the dependent routes.JIRA Issue