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 #304

Closed
wants to merge 8 commits into from
Closed

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Mar 16, 2017

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

This is a WIP PR because:

Includes

  • 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

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

Still to do

  • Examples in content schema updated, tests written against them
  • Integration tests
  • Add Airbrake notification when:
    • finder is missing
    • a value is provided that is now in the allowed list
    • A date is provided that's not parseable
      the date did not get done, the current behaviour is to 500 in general, and i think that is correct, generally with the state of monitoring, i think we should 500 on unexpected so it definitely gets picked up.

Notes

  • The random content generator doesn't appear to create random facets and values
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 16, 2017 Inactive
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 21, 2017 Inactive
@nickcolley nickcolley force-pushed the filterable-specialist-docs branch from c8158d5 to 802566e Mar 21, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 21, 2017 Inactive
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 30, 2017 Inactive
@mcgoooo mcgoooo force-pushed the filterable-specialist-docs branch from cbb22a1 to b431333 Mar 30, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 30, 2017 Inactive
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 30, 2017 Inactive
@mcgoooo mcgoooo force-pushed the filterable-specialist-docs branch from a51e0e0 to c74270b Mar 30, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 30, 2017 Inactive
@mcgoooo mcgoooo changed the title [WIP] Add facet metadata to specialist documents Add facet metadata to specialist documents Mar 30, 2017
@mcgoooo mcgoooo force-pushed the filterable-specialist-docs branch from c74270b to 504a8cb Mar 31, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 31, 2017 Inactive
@mcgoooo mcgoooo force-pushed the filterable-specialist-docs branch from 504a8cb to a591533 Mar 31, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-304 Mar 31, 2017 Inactive
fofr and others added 8 commits Mar 15, 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
Andrew Hilton
end


This comment has been minimized.

@fofr

fofr Apr 6, 2017
Author Contributor

Rogue whitespace.

assert_equal history.first["note"], @content_item["details"]["change_history"].last["note"]
assert_equal history.last["note"], @content_item["details"]["change_history"].first["note"]
assert_equal history.size, @content_item["details"]["change_history"].size
end
end


This comment has been minimized.

@fofr

fofr Apr 6, 2017
Author Contributor

More rogue whitespace.

def finder
content_item.dig("links", "finder", 0)
# TODO: Shout loudly if missing parent finder
first_finder = content_item.dig("links", "finder", 0)

This comment has been minimized.

@fofr

fofr Apr 6, 2017
Author Contributor

I'd split the Airbrake commit from the refactoring.

not_allowed = "facet value not in list of allowed values"

values.each do |v|
Airbrake.notify(not_allowed) unless allowed_values.include?(v)

This comment has been minimized.

@fofr

fofr Apr 6, 2017
Author Contributor

We should include the not allowed value in the Airbrake notification for easier debugging.

@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 6, 2017

What's the status of this PR?

["<a href=\"/countryside-stewardship-grants?land_use%5B%5D=arable-land\">Arable land</a>",
"<a href=\"/countryside-stewardship-grants?land_use%5B%5D=wildlife-package\">Wildlife package</a>",
"<a href=\"/countryside-stewardship-grants?land_use%5B%5D=water-quality\">Water quality</a>",
"<a href=\"/countryside-stewardship-grants?land_use%5B%5D=wildlife-package\">Wildlife package</a>"].join(", ")

This comment has been minimized.

@fofr

fofr Apr 6, 2017
Author Contributor

Why is wildlife package included twice?

@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 13, 2017

Closing while I finish up this work.

@fofr fofr closed this Apr 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.