-
Notifications
You must be signed in to change notification settings - Fork 17
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
Refactor Get Involved #2712
Refactor Get Involved #2712
Conversation
bc29e8a
to
1bf6d03
Compare
This page was created differently to all the other content types. Refactoring so it is the same as everything else. Co-authored-by: Jon Hallam <[email protected]>
1bf6d03
to
f21a0ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, nice improvement ⭐
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this Bruce, looks great.
It's a shame the review app doesn't work - is that something to do with search? - but I checked it out locally using the start up script (retro!) and it all looked great.
Yes, I think it's because this app's Heroku setup doesn't include the env var to provide the URL for live search. This is the only document type in Government Frontend that uses search, so maybe it's not the right app for the document to be rendered by 🤔. |
link: { | ||
text: item["title"], | ||
path: item["link"], | ||
description: "#{close_status} #{item['end_date'].to_date.strftime('%d %B %z')}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, too late!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot! I don't know how that changed, since I just copy and pasted the stuff over 🤷♂️.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll raise a PR to fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was incorrectly added in #2712, so fixing to display the year instead of the timezone.
This fixes the review apps #2715 |
This page was created differently to all the other content types.
Refactoring so it is the same as everything else.
The test coverage isn't as good as it should be, but retains the same level of testing as before the refactor.
Tests depend on alphagov/publishing-api#2312.
Follow these steps if you are doing a Rails upgrade.