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

Render specialist documents #265

Merged
merged 3 commits into from Mar 3, 2017
Merged

Render specialist documents #265

merged 3 commits into from Mar 3, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Mar 1, 2017

https://trello.com/c/h9hITGPy/158-render-specialist-content-in-government-frontend
Tests depend on merge and deploy of alphagov/govuk-content-schemas#548

This is the first part of switching specialist content to government-frontend

  • Renders with an incomplete metadata and footer
  • Renders with existing contents list rather than the nested version currently on specialist frontend
  • Use the oldest entry in the change history to determine first published date – this matches specialist frontend but should be updated when https://trello.com/c/xCJ3RN6W/ is ready
  • Display change history in reverse chronological order, matching all other formats

Note: This doesn't include recently added button support: alphagov/specialist-frontend#138

Screenshots

Old New
cma-cases-old cma-cases-new
aaib-old aaib-new
@fofr fofr force-pushed the specialist-documents branch from e848191 to ccec7de Mar 1, 2017
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-265 Mar 1, 2017 Inactive

def first_public_at
content_item["details"]["first_public_at"]
end

This comment has been minimized.

@mcgoooo

mcgoooo Mar 1, 2017
Contributor

just wondering if we should just use the specialist document presenter way of doing this everywhere so we don't hit that snag again until we have a reliable fix? question more than comment

This comment has been minimized.

@fofr

fofr Mar 1, 2017
Author Contributor

All other formats that display published/updated uses first_public_at at the moment. I think it's ok to say "specialist is different" and leave as-is for now. Might need to re-consider when we bring in other formats.

When the data upstream is fixed we'll only need to change it in two places.

This comment has been minimized.

@mcgoooo

mcgoooo Mar 1, 2017
Contributor

👍

@mcgoooo
mcgoooo approved these changes Mar 1, 2017
Copy link
Contributor

@mcgoooo mcgoooo left a comment

Generally looks sound to me

</div>
</div>

<%= render 'shared/footer', @content_item.document_footer %>

This comment has been minimized.

@mcgoooo

mcgoooo Mar 1, 2017
Contributor

i presume we will be handling the normalisation of this and travel advice later?

This comment has been minimized.

@fofr

fofr Mar 1, 2017
Author Contributor

Travel advice doesn't have a footer.

But yes, normalisation between the similar contacts and travel advice can happen when all has been merged.

This comment has been minimized.

@fofr

fofr Mar 1, 2017
Author Contributor

I prefer to do those refactors as separate PRs

This comment has been minimized.

@mcgoooo

mcgoooo Mar 1, 2017
Contributor

yep no worries 👍


def any_updates?
if (first_public_at = content_item["details"]["first_public_at"]).present?
if first_public_at.present?

This comment has been minimized.

@bevanloon

bevanloon Mar 3, 2017
Contributor

This implies that first_public_at might not exist. I don't think we have an explicit test for this case?
If we can get a situation of not having first_public_at we should perhaps test for it. If we never get a situation where that can happen then we can do away with the conditional here.

This comment has been minimized.

@h-lame

h-lame Mar 3, 2017
Contributor

It can be blank for first drafts (I don't know if government-frontend is used to preview content in the draft stack though).

fofr added 3 commits Dec 6, 2016
* Renders with an incomplete metadata and footer
  * Metadata pending https://trello.com/c/6CBttvZL/
* Render with existing contents list
The order of change history isn’t guaranteed:
alphagov/govuk-content-schemas#545

* Use the provided timestamp to order by date
* Update test to use parseable dates
* first_published_at does not have reliable data, at time of writing
dates could be after public_updated_at
* the standard approach using details[“first_public_at”] is not
available
* Instead use first date in change history

When https://trello.com/c/xCJ3RN6W/ has been fixed, and when the dates
are reliable this can be switched back.

Add some tests to updatable to be explicit about expected behaviour
around presence of first_public_at
@fofr fofr force-pushed the specialist-documents branch from ccec7de to 592dc41 Mar 3, 2017
@fofr fofr merged commit 573329b into master Mar 3, 2017
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
security/snyk No new vulnerabilities
Details
@fofr fofr deleted the specialist-documents branch Mar 3, 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

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