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

Remove publication templates #3370

Merged
merged 14 commits into from
Aug 3, 2017
Merged

Remove publication templates #3370

merged 14 commits into from
Aug 3, 2017

Conversation

tuzz
Copy link
Contributor

@tuzz tuzz commented Aug 2, 2017

https://trello.com/c/kfyYY4tf/73-9-publications-migration-final-tasks-deploy-2-05-days

The rendering of these pages is now handled by government-frontend. This work removes this code from Whitehall and re-works some of the tests to equivalently test the things where assertions were previously made against these views.

It's probably easiest to review this commit-by-commit and read the commit messages.

The supersedes #3136 which deleted more code than was necessary.

@tuzz tuzz force-pushed the remove-publication-templates branch 2 times, most recently from 7e82885 to b3c3b56 Compare August 2, 2017 15:35
@rubenarakelyan
Copy link
Contributor

🎉

Nice

screen shot 2017-08-03 at 09 28 10


<div class="block-2 heading-block">
<div class="inner-block">
<%= render partial: "documents/attachment_full_width",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this partial also be removed? Looks like it was only referenced on publications and statistics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed and rebased.

@fofr
Copy link
Contributor

fofr commented Aug 3, 2017

Just the one thing I've found that could also be removed.

👍 💯

tuzz added 14 commits August 3, 2017 10:25
We can no longer test these changes are reflected
on the show page, but we can still check the data
has changed.
We can no longer test these changes are reflected
on the show page, but we can still check the data
has changed.
This feature is about to be removed as it will be
handled via the draft stack. This feature was
failing after removing publications#show there’s
not much point fixing it.
We can no longer test these changes are reflected
on the show page, but we can still check the data
has changed.
We can no longer test these changes are reflected
on the show page, but we can still check the data
has changed.
We converted this to an admin feature that checks
the data has been updated correctly in the
database.
We converted this to an admin feature that checks
the data has been updated correctly in the
database.
We converted this to an admin feature that checks
the data has been updated correctly in the
database.
Attachments are far more thoroughly tested in
features/edition-attachments.feature. The feature
we removed relied on publications#show which no
longer exists.
These features don’t interact with Whitehall
admin, they directly set up data then visit the
publications#show page to assert things.

This page is now being served by government
frontend so these feature can be removed.
@tuzz tuzz force-pushed the remove-publication-templates branch from b3c3b56 to 69ebea7 Compare August 3, 2017 10:26
@tuzz
Copy link
Contributor Author

tuzz commented Aug 3, 2017

Deployed to integration. Everything looks ok. Merging.

@tuzz tuzz merged commit 1784672 into master Aug 3, 2017
@tuzz tuzz deleted the remove-publication-templates branch August 3, 2017 10:37
floehopper added a commit that referenced this pull request May 16, 2023
These shared tests seem to have become redundant in this commit [1] as
part of this PR [2].

[1]: abee1a7
[2]: #3370
floehopper added a commit that referenced this pull request May 16, 2023
These shared tests seem to have become redundant in this commit [1] as
part of this PR [2].

[1]: abee1a7
[2]: #3370
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants