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

Add facet metadata to specialist documents #324

Merged
merged 9 commits into from Apr 18, 2017
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 13, 2017

https://trello.com/c/al2fPH5p/192-1-render-missing-finder-metadata-in-government-frontend

  • Add facet values to metadata and document footer now they're provided by the finder in the links hash. Assumes that all possible facet values could be multiple and so normalizes values as an array before working with them. Multiple values get joined with a comma.
  • Add custom breadcrumb based on finder
  • Fix logic for display of updated field in metadata and footer
  • Sends Airbrake notifications when missing finder or forbidden facet value found
  • The random content generator doesn't create random facets and values

How facets display in metadata

Facet type Allowed values Filterable Displays
Text True True Shows the allowed value labels with links to the filtered results
Text True False Shows the allowed value labels, no links
Text False - Shows the provided text directly, never linked
Date - - Shows a friendly date, never linked

Screenshots

Feature Old New
Metadata screen shot 2017-04-13 at 12 30 38 screen shot 2017-04-13 at 12 30 23
Document footer screen shot 2017-04-13 at 12 30 52 screen shot 2017-04-13 at 12 30 45
fofr and others added 9 commits Apr 13, 2017
* Merge facet values within details[‘metadata’] with facets provided by
the single finder in the links hash
* When text and no allowed values, display text value
* When text with allowed values, convert to friendly version
* When a text field with allowed values is filterable, link to the
filtered results
* When showing a date facet, render a friendly date

Examples can’t be updated with facets because of:
alphagov/govuk-content-schemas#453

* Use test fixtures instead of examples for now
Specialist documents don’t have a parent because of:
alphagov/publishing-api#835

For now, use a custom one based on the finder provided in the links
hash, if available.
Dates need to render in the left column of the document footer
component, they get passed in as a separate hash.

* Use separate logic for metadata and document footer components
Specialist document change history can have a modified date that is
slightly different to the public_updated_at, eg milliseconds different

This means the direct comparison in updatable gives a false positive

Use change_history as specialist-frontend did
the specialist presenter is a bit messy due to the content item
this hopefully makes it (slightly) clearer, also add in some airbrake
for when data is not as we expect
When content has a facet value that is not in the distinct set of
allowed values, rather than falling over and giving the user a 500,
instead render the value as it has been provided. Send a notification
to Errbit that there is an invalid value being presented.

Matches existing specialist-frontend behaviour.

Follows principles of:
1. Frontend rendering as best it can whatever has been provided by
content-store
2. Shouting (at least a little), when that content is invalid
Add explicit test showing behaviour when a details hash contains
reference to a facet and value that is not provided by its parent
finder.

(eg Consider a finder facet list is edited but the details of child
specialist documents are not updated)
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-324 Apr 13, 2017 Inactive
@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Apr 18, 2017

Done a wraith run on top pages: http://furry-dog.surge.sh/

FYI, there's some wrapping issues here: http://furry-dog.surge.sh/specialist_documents_1/MULTI_phantomjs_heroku.png which is address by the metadata improvements we're going to make so shouldnt block this.

@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Apr 18, 2017

@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 18, 2017

@nickcolley:

  • Regarding the wrapping, I think that's ok for now. As you said we'll be changing this soon.
  • Regarding the "full page history", the format now behaves like all the others – it only shows the change history when updates have been made. This means it omits a disclosure that reveals "First published." – ie redundant information.

@nickcolley nickcolley merged commit 21947d5 into master Apr 18, 2017
2 checks passed
@nickcolley nickcolley deleted the filterable-specialist-docs branch Apr 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants