Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPP12087 help centre structure refactor #385

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

SPP12087 help centre structure refactor #385

wants to merge 12 commits into from

Conversation

JasonBellONS
Copy link
Collaborator

Help centre refactored to no longer pull in the structure page from contentful, structure and navigation built on demand using information from the other help centre pages grabbed from contentful

… to build the help centre navigation and build on demand instead

Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
…ilt each time a help centre page is accessed

Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
@JasonBellONS
Copy link
Collaborator Author

Note failing tests due to changes in ordering of the content, should be resolved under https://jira.ons.gov.uk/browse/SPP-11740

Copy link
Collaborator

@gibbardsteve gibbardsteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reviewed the code yet, but could we try and maintain URLs. When I load help centre pages in the dev server they have a different URL to what we currently had deployed. E.g:

Find and view methods
prod :
image

dev :
image

If you could check the pages and try and get them to align that'd be good.

… consistent

Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
@JasonBellONS
Copy link
Collaborator Author

Not reviewed the code yet, but could we try and maintain URLs. When I load help centre pages in the dev server they have a different URL to what we currently had deployed. E.g:

Find and view methods prod : image

dev : image

If you could check the pages and try and get them to align that'd be good.

Hi, this should now be fixed

Copy link
Collaborator

@gibbardsteve gibbardsteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions and some minor comments.

Also I notice the live site doesn't have Help Centre Index in Alphabetical order:

image

I vaguely recall this may have been a bug we noticed in the last deploy that was since fixed (i.e made alphabetical) - just checking the change is expected?

sml_builder/__init__.py Show resolved Hide resolved
sml_builder/__init__.py Show resolved Hide resolved
"label": "Submit a method request",
}
)
nav["categories"] = sorted(nav["categories"], key=lambda x: x["name"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a comment here just to call out sorting is done in code (rather than in contentful)

category["subcategories"] = sorted(
category["subcategories"], key=lambda x: x["label"]
)
app.cache["help_centre_nav"] = nav
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this cache live? If someone navigates to the website and we deploy a new version of the website how does the cache get refreshed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cache is a dictionary object within the flask application object, each time we deploy that application will be recreated, so in the case of this method we deploy, and during the first request to it we create this navigation and add it to the cache dictionary, it will remain here unless deleted or the app is stopped

sml_builder/__init__.py Show resolved Hide resolved
@JasonBellONS
Copy link
Collaborator Author

A few questions and some minor comments.

Also I notice the live site doesn't have Help Centre Index in Alphabetical order:

image

I vaguely recall this may have been a bug we noticed in the last deploy that was since fixed (i.e made alphabetical) - just checking the change is expected?

If i remember correctly it was the methods page that was made alphabetical, i made a similar change here as you can't ensure that content is gathered in a specific order, my thoughts were displaying it alphabetically made most sense

Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
@JasonBellONS
Copy link
Collaborator Author

@gibbardsteve comments added where requested

Copy link
Collaborator

@gibbardsteve gibbardsteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the build step has failed?

image

sml_builder/__init__.py Show resolved Hide resolved
@JasonBellONS
Copy link
Collaborator Author

Oh right yes thats my mistake, during the freeze process the freeze module would make requests to each page in order to build that page and freeze it, so this process would be happening during that build step

Signed-off-by: jasonbellONS <jason.bell@ons.gov.uk>
@ons-spp-machine-user
Copy link
Collaborator

DEPLOY_URL=https://d1uzhgtw86gcqp.cloudfront.net/
cloudfront_id=EEJOC5TWG5769

Copy link
Collaborator

@gibbardsteve gibbardsteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. We may want to hold off building prod until we get a green light from MQD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants