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

Use separate content items for start page and form #185

Merged
merged 3 commits into from Jun 13, 2017
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jun 7, 2017

The start page will be moving to Publisher/Frontend. This is the first step towards that.

  • The form will continue to be rendered by calculators and needs its own content item
    • This content item will have none of the tagged taxons/topics/links
    • The content item will not be sent to rummager, users should start from the start page
  • Use exact routes rather than a prefix to separate start page and form
  • Create a new content item for the form that gets published on deploy
  • Omit public_updated_at from payload to avoid discrepancies between first_published_at and public_updated_at.

Calculators currently reads from this hardcoded content_item: https://github.com/alphagov/calculators/blame/two-content-items/app/controllers/child_benefit_tax_controller.rb#L35

Splitting content items has no affect on what's currently rendered.

cc @kevindew @tijmenb @carvil

@fofr fofr changed the title [Do not merge] Use separate content items for start page and form Use separate content items for start page and form Jun 7, 2017
class CalculatorFormContentItem < CalculatorContentItem
attr_reader :calculator

def base_path

This comment has been minimized.

@carvil

carvil Jun 7, 2017
Contributor

Should the new form page have the same title as the start page?

This comment has been minimized.

@fofr

fofr Jun 7, 2017
Author Contributor

I think so, both pages have the same title at the moment.

fofr added a commit to alphagov/publisher that referenced this pull request Jun 8, 2017
* The path needs to be claimed so that Publisher can publish to it
  (this will stop Calculators from publishing to that path too)
* Draft a start page with content that's currently live
* Delete any old artefact owned by calculators so new artefact can be created
  * Maintain Need IDs
  * Use existing content_id so that content doesn't need to be unpublished
    (which would risk users not seeing the page)

Steps to switch the start page:
* Merge and deploy: alphagov/calculators#185
* Run `claim` rake task
* Run `draft` rake task
* Preview draft, send to review, skip review then publish
fofr added a commit to alphagov/publisher that referenced this pull request Jun 8, 2017
* The path needs to be claimed so that Publisher can publish to it
  (this will stop Calculators from publishing to that path too)
* Draft a start page with content that's currently live
* Delete any old artefact owned by calculators so new artefact can be created
  * Maintain Need IDs
  * Use existing content_id so that content doesn't need to be unpublished
    (which would risk users not seeing the page)

Steps to switch the start page:
* Merge and deploy: alphagov/calculators#185
* Run `claim` rake task
* Run `draft` rake task
* Preview draft, send to review, skip review then publish
@fofr
Copy link
Contributor Author

@fofr fofr commented Jun 8, 2017

Closing for now: Calculators depends on the route: /child-benefit-tax-calculator/process_form – after this change that route does not make it to the calculators app and 404s.

Fixed in da22fd9

@fofr fofr closed this Jun 8, 2017
@fofr fofr reopened this Jun 8, 2017
@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Jun 9, 2017

Do we need to think about any redirects here? I imagine no one is linking directly to the process_form endpoint

@fofr
Copy link
Contributor Author

@fofr fofr commented Jun 9, 2017

@nickcolley My assumption was not. Any links should be to main?... with the params attached to the end.

@fofr fofr requested review from tijmenb, kevindew and carvil Jun 12, 2017
Copy link
Member

@kevindew kevindew left a comment

Looks good to me 👍

fofr added 3 commits Jun 6, 2017
The start page will be moving to Publisher/Frontend. This is the first
step toward that.

* The form will continue to be rendered by calculators
* Use exact routes rather than a prefix
* Creates a new content item for the form part of the calculator that
gets published on deploy
Publishing API will default to the time the new edition is published.

When creating the new content item for the form, this time was setting
`public_updated_at` but `first_published_at` was omitted – leading to
the Publishing API defaulting to publish time – which was 1 second
later than `public_updated_at`.

Removing `public_updated_at` from the payload avoids this discrepancy.

It also means `public_updated_at` is not updated every time there is a
deploy, as it does not change when the update is minor.
* Allow /main/process_form route to be processed by calculators app
@fofr fofr force-pushed the two-content-items branch from da22fd9 to 04139c7 Jun 13, 2017
@fofr fofr merged commit 186e088 into master Jun 13, 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 two-content-items branch Jun 13, 2017
@fofr fofr mentioned this pull request Jun 13, 2017
2 of 2 tasks complete
fofr added a commit to alphagov/publisher that referenced this pull request Jun 13, 2017
* The path needs to be claimed so that Publisher can publish to it
  (this will stop Calculators from publishing to that path too)
* Draft a start page with content that's currently live
* Delete any old artefact owned by calculators so new artefact can be created
  * Maintain Need IDs
  * Use existing content_id so that content doesn't need to be unpublished
    (which would risk users not seeing the page)

Steps to switch the start page:
* Merge and deploy: alphagov/calculators#185
* Run `claim` rake task
* Run `draft` rake task
* Preview draft, send to review, skip review then publish
fofr added a commit to alphagov/publisher that referenced this pull request Jun 13, 2017
* The path needs to be claimed so that Publisher can publish to it
  (this will stop Calculators from publishing to that path too)
* Draft a start page with content that's currently live
* Delete any old artefact owned by calculators so new artefact can be created
  * Maintain Need IDs
  * Use existing content_id so that content doesn't need to be unpublished
    (which would risk users not seeing the page)

Steps to switch the start page:
* Merge and deploy: alphagov/calculators#185
* Run `claim` rake task
* Run `draft` rake task
* Preview draft, send to review, skip review then publish
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

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