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 web_isbn and print_meta_data_contact_address #1649

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

JonathanHallam
Copy link
Contributor

@JonathanHallam JonathanHallam commented Feb 10, 2020

Trello - https://trello.com/c/w5dsqKuc/1381-remove-redundant-fields-for-whitehall-attachments-org-contact-and-web-isbn

These file types are redundant and should be removed. They are no longer being sent to the publishing api by whitehall

These fields are also being removed from govuk content schemas and whitehall
whitehall - alphagov/whitehall#5333
content schemas - alphagov/govuk-content-schemas#958


Visual regression results:
https://government-frontend-pr-[THIS PR NUMBER].surge.sh/gallery.html

@bevanloon bevanloon temporarily deployed to government-f-remove-red-pr82hw February 10, 2020 14:08 Inactive
@JonathanHallam JonathanHallam force-pushed the remove-redundant-fields branch from 4999755 to c11927a Compare February 10, 2020 14:16
@bevanloon bevanloon temporarily deployed to government-f-remove-red-pr82hw February 10, 2020 14:16 Inactive
@JonathanHallam JonathanHallam force-pushed the remove-redundant-fields branch from c11927a to f7d1fa4 Compare February 10, 2020 14:17
@bevanloon bevanloon temporarily deployed to government-f-remove-red-pr82hw February 10, 2020 14:17 Inactive
Trello - https://trello.com/c/w5dsqKuc/1381-remove-redundant-fields-for-whitehall-attachments-org-contact-and-web-isbn

These file types are redundant and should be removed. They are no longer being sent to the publishing api by whitehall
Copy link
Contributor

@benthorner benthorner left a comment

Choose a reason for hiding this comment

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

👍

Any enquiries regarding this publication should be sent to us at:<br/>
</p>
<%= simple_format(@content_item.print_meta_data_contact_address) %>
<% end %>

<% unless @content_item.isbn.blank? %>
<p>
Print ISBN: <%= @content_item.isbn %>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would make sense to remove Print here, although that would be a user-facing change. Perhaps raise in a separate PR, as it's confusing to see it used without context of knowing there used to be another ISBN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good point, I'll raise a separate one after this :)

@JonathanHallam JonathanHallam merged commit 5fc91a0 into master Feb 11, 2020
@JonathanHallam JonathanHallam deleted the remove-redundant-fields branch February 11, 2020 12:53
JonathanHallam added a commit that referenced this pull request Feb 11, 2020
Following #1649 Web ISBN has been removed. This left us with "Print ISBN" which sounds a bit odd if there are no other ISBN's. This renames "Print ISBN" to make things a bit clearer.
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.

3 participants