Unpublish edition edit #2374
Unpublish edition edit #2374
Conversation
syed-ali-tw
force-pushed
the
unpublish-edition-edit
branch
from
95bee22
to
172fed4
Compare
ellohez
force-pushed
the
unpublish-edition-edit
branch
from
7f760d5
to
3251109
Compare
syed-ali-tw
force-pushed
the
unpublish-edition-edit
branch
from
ea0340b
to
c67e98f
Compare
co-authored-by: Helen Pickavance<[email protected]>
Co-authored-by: Helen Pickavance <[email protected]>
Co-authored-by: Helen Pickavance <[email protected]>
- Published and draft tests separated for TabbedNavHelperTest - Unpublish tab removed from editions_controller_test - awaiting investigation - Add div class to fix layout to bottom tab section - Correct linting/rubocop errors
syed-ali-tw
force-pushed
the
unpublish-edition-edit
branch
5 times, most recently
from
94d17de
to
f58c6cd
Compare
ellohez
force-pushed
the
unpublish-edition-edit
branch
from
f58c6cd
to
a08b68f
Compare
| <% end %> | ||
| <% end %> | ||
|
|
||
| <%= render "govuk_publishing_components/components/inset_text", { |
There was a problem hiding this comment.
This component needs to be inside a govuk-grid-row wrapper otherwise it is wider than other content. Moving into the one below should be OK.
| <%= form_for @edition, url: process_unpublish_edition_path(@edition), method: :post do %> | ||
| <%= hidden_field_tag :redirect_url, params[:redirect_url] %> | ||
| <p class="govuk-body govuk-!-margin-bottom-7">Are you sure you want to unpublish this document?</p> | ||
| <div class="govuk-button-group govuk-!-margin-bottom-6"> |
There was a problem hiding this comment.
We wouldn't need govuk-!-margin-bottom-6 here to be consistent with the main "Unpublish" page.
|
Just added a couple of small comments regarding the UI but otherwise this looks good to me. |
| def process_unpublish | ||
| artefact = @resource.artefact | ||
|
|
||
| success = params["redirect_url"].strip.empty? ? UnpublishService.call(artefact, current_user) : UnpublishService.call(artefact, current_user, redirect_url) |
There was a problem hiding this comment.
For readability - maybe worth cleaning it up a little, setting success to a method defined outside of process_unpublish method.
| end | ||
| rescue StandardError | ||
| @resource.errors.add(:unpublish, downstream_error_message) | ||
| render "secondary_nav_tabs/confirm_unpublish" |
There was a problem hiding this comment.
We could avoid duplication of those two lines
@resource.errors.add(:unpublish, downstream_error_message)
render "secondary_nav_tabs/confirm_unpublish"
by extracting them into separate method
|
|
||
| should "redirect to edition_path when user does not have govuk-editor permission" do | ||
| user = FactoryBot.create(:user, :welsh_editor, name: "Stub User") | ||
| login_as(user) |
There was a problem hiding this comment.
Could we move it to the setup,
def login_as_welsh_editor
user = FactoryBot.create(:user, :welsh_editor, name: "Stub User")
login_as(user)
end
to avoid repetitions below
There was a problem hiding this comment.
that would mean we will have to create govuk-editor user for other test, so I am not sure if that would reduce repitations
syed-ali-tw
force-pushed
the
unpublish-edition-edit
branch
from
182db78
to
d20ad3e
Compare
| <% end %> | ||
| <% end %> | ||
|
|
||
| <div class="govuk-grid-row"> |
There was a problem hiding this comment.
Looks like this row is nested inside another. Probably for this view we can put everything in a single row.
syed-ali-tw
force-pushed
the
unpublish-edition-edit
branch
from
ac71f6b
to
d3cea9f
Compare
Trello
Add unpublish functionality for new design system edit page to allow unpublishing of published edition.
When edition is draft, does not shows unpublish tab
When edition is published, shows unpublish tab
Confirm unpublish
Follow these steps if you are doing a Rails upgrade.