Skip to content

Commit

Permalink
Return to the edit mapping page after save
Browse files Browse the repository at this point in the history
* Update messaging on create to give feedback about type or mapping,
the canonicalised path and the redirect location if appropriate
* Redirect back to mapping edit screen so user can quickly confirm
their changes or fix mistakes
  • Loading branch information
Paul Hayes committed Nov 20, 2013
1 parent 9145a6a commit 0106e19
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 6 deletions.
10 changes: 8 additions & 2 deletions app/controllers/mappings_controller.rb
Expand Up @@ -14,7 +14,13 @@ def new
def create
@mapping = @site.mappings.build(params[:mapping])
if @mapping.save
redirect_to site_mappings_path(@site), notice: 'Mapping saved.'
if @mapping.redirect?
link = ActionController::Base.helpers.link_to @mapping.new_url, @mapping.new_url

This comment has been minimized.

Copy link
@rgarner

rgarner Nov 21, 2013

Contributor

I wouldn't normally expect to see a call to ActionController::Base.helpers.link_to in here - that would be more of a view concern. However, flash[:notice] overlaps a bit because you're having to set it in the controller. I think you can avoid the ugliness of the ActionController::Base reference by using view_context.link_to instead. If you're going to do that, I think you can move the whole if @mapping.redirect? block to MappingsHelper#created_notice, and just say redirect_to edit_site_mapping_path(@site, @mapping), notice: view_context.created_notice(@mapping)

notice = "Mapping created. <strong>#{@mapping.path}</strong> redirects to <strong>#{link}</strong>"
else
notice = "Mapping created. <strong>#{@mapping.path}</strong> has been archived"
end
redirect_to edit_site_mapping_path(@site, @mapping), notice: notice.html_safe
else
render action: 'new'
end
Expand All @@ -31,7 +37,7 @@ def edit
def update
@mapping = @site.mappings.find(params[:id])
if @mapping.update_attributes(params[:mapping])
redirect_to site_mappings_path(@site), notice: 'Mapping saved.'
redirect_to edit_site_mapping_path(@site, @mapping), notice: 'Mapping saved.'
else
render action: 'edit'
end
Expand Down
5 changes: 3 additions & 2 deletions features/mapping_create.feature
Expand Up @@ -11,9 +11,10 @@ Feature: Create a mapping
Then I should see "http://bis.gov.uk"
When I make the mapping a redirect from /Needs/Canonicalizing/q=1 to http://gov.uk/organisations/bis
And I create the mapping
Then I should be returned to the mappings list for bis
And I should see "Mapping saved."
Then I should be returned to the edit mapping page with a success message
And I should see "Mapping created."
And I should see "/needs/canonicalizing"
And I should see "http://gov.uk/organisations/bis"

Scenario: I don't have access
Given I have logged in as a member of another organisation
Expand Down
4 changes: 2 additions & 2 deletions features/mapping_edit.feature
Expand Up @@ -18,9 +18,9 @@ Feature: Edit a site's mapping
Then I should see redirect fields
But I should not see archive fields
When I save the mapping
Then I should be returned to the mappings list for bis
Then I should be returned to the edit mapping page with a success message
And I should see "Mapping saved"
And I should see "https://gov.uk/new-url"
And I should be editing the mapping for "/about"

@javascript
Scenario: Editing a site mapping that is an archive
Expand Down
9 changes: 9 additions & 0 deletions features/step_definitions/mappings_assertion_steps.rb
Expand Up @@ -6,6 +6,15 @@
step 'I should see "Edit mapping"'
end

Then(/^I should be editing the mapping for "([^"]*)"$/) do |path|
expect(page).to have_selector("form a[href*='#{path}']")
end

Then(/^I should be returned to the edit mapping page with a success message$/) do
step 'I should see "Edit mapping"'
page.should satisfy {|page| page.has_content?('Mapping created') or page.has_content?('Mapping saved')}
end

Then(/^the filter box should contain "([^"]*)"$/) do |path|
expect(page).to have_field('Filter by path', with: path)
end
Expand Down

0 comments on commit 0106e19

Please sign in to comment.