134929249 two flash messages bug

app/controllers/proposed_organisations_controller.rb

@@ -8,10 +8,9 @@ def index
   end
 
   def update
-    proposed_org = ProposedOrganisation.friendly.find params[:id]
-    result = AcceptProposedOrganisation.new(proposed_org).run
-    set_flash_for_accepting_proposed_org result
-    redirect_to organisation_path(result.accepted_organisation) and return false
+    rslt = prepare_data_for_update
+    redirect_to organisation_path(rslt.accepted_org) and return false if rslt.accepted_org
+    redirect_to proposed_organisations_path
   end
 
   def destroy
@@ -34,7 +33,6 @@ def create
     else
       render :new
     end
-
   end
 
   def show
@@ -46,22 +44,12 @@ def show
   end
 
   private
-
-  def set_flash_for_accepting_proposed_org result
-    flash[:notice] = "You have approved the following organisation"
-    flash_msg = case result.status
-      when AcceptProposedOrganisation::Response::INVALID_EMAIL
-        "No invitation email was sent because the email associated with #{result.accepted_organisation.name}, #{result.accepted_organisation.email}, seems invalid"
-      when AcceptProposedOrganisation::Response::NO_EMAIL
-        "No invitation email was sent because no email is associated with the organisation"
-      when AcceptProposedOrganisation::Response::INVITATION_SENT
-        "An invitation email was sent to #{result.accepted_organisation.email}"
-      when AcceptProposedOrganisation::Response::NOTIFICATION_SENT
-        "A notification of acceptance was sent to #{result.accepted_organisation.email}"
-      else
-        "No mail was sent because: #{result.error_message}"
-      end
-     flash[:error] = flash_msg
+
+  def prepare_data_for_update
+    proposed_org = ProposedOrganisation.friendly.find params[:id]
+    rslt = AcceptProposedOrganisation.new(proposed_org).run
+    flash.replace(CreateFlashForProposedOrganisation.new(rslt).run)
+    rslt
   end
 
   def make_user_into_org_admin_of_new_proposed_org
@@ -112,4 +100,4 @@ def self.build params
       category_ids: []
     )
   end
-end
+end

app/models/organisation.rb

@@ -30,6 +30,12 @@ def update_attributes_with_superadmin(params)
     return unless email.blank? || can_add_or_invite_admin?(email)
     self.update_attributes(params)
   end
+
+  def rollback_acceptance
+    org = becomes!(ProposedOrganisation)
+    org.save!
+    org
+  end
 
   def self.search_by_keyword(keyword)
      keyword = "%#{keyword}%"

app/models/proposed_organisation.rb

@@ -1,5 +1,6 @@
 class ProposedOrganisation < BaseOrganisation
   has_many :users, :foreign_key => 'organisation_id'
+
   def accept_proposal
     org = becomes!(Organisation)
     org.save!

app/services/accept_proposed_organisation.rb

@@ -6,42 +6,87 @@ class Response
     NO_EMAIL = "No email"
     OTHER_ERROR = "Other Error"
 
-    attr_reader :status, :error_message, :accepted_organisation
+    attr_reader   :status, :error_msg 
+    attr_accessor :accepted_org, :not_accepted_org
 
-    def initialize status, error_message, accepted_organisation
+    def initialize status, error_msg, accepted_organisation
       @status = status
-      @error_message = error_message
-      @accepted_organisation = accepted_organisation
+      @error_msg = error_msg
+      @accepted_org = accepted_organisation
+      @not_accepted_org = nil
     end
   end
+
+  class NotificationError < StandardError; end
 
-  def initialize(proposed_org)
-    @proposed_org = proposed_org
-    @email = @proposed_org.email
+  def initialize proposed_org
+    @org = proposed_org
+    @email = @org.email
   end
 
   def run
-    org = @proposed_org.accept_proposal
-    usr = User.find_by(email: @email)
-    if usr
-      NotifyRegisteredUserFromProposedOrg.new(usr,org).run
-      Response.new(Response::NOTIFICATION_SENT,nil, org)
-    else
-      create_invitation_response_object(InviteUnregisteredUserFromProposedOrg.new(@email,org).run, org)
-    end
+    accept_organisation
+  rescue NotificationError
+    rollback_changes_to_org
   end
 
   private
-
-  def create_invitation_response_object(result_of_inviting, org)
-    return Response.new(Response::INVITATION_SENT, result_of_inviting.error_message, org) if result_of_inviting.success?
-    case result_of_inviting.status
-      when InviteUnregisteredUserFromProposedOrg::Response::INVALID_EMAIL
-        Response.new(Response::INVALID_EMAIL, result_of_inviting.error_message, org)
-      when InviteUnregisteredUserFromProposedOrg::Response::NO_EMAIL
-        Response.new(Response::NO_EMAIL, result_of_inviting.error_message, org)
-      else
-        Response.new(Response::OTHER_ERROR, result_of_inviting.error_message, org)
-    end
+
+  def accept_organisation
+    @org = @org.accept_proposal
+    @result = inform_user
+    raise NotificationError unless success?(@result.status)
+    @result                    
+  end
+
+  def create_invitation_response_object rslt_of_inviting
+    response_obj(response_type(rslt_of_inviting), rslt_of_inviting.error_msg)
+  end
+
+  def inform_user
+    @usr = User.find_by(email: @email)
+    return notify_registered_usr if @usr
+    rslt = InviteUnregisteredUserFromProposedOrg.new(@email, @org).run
+    create_invitation_response_object(rslt)
+  end
+
+  def invalid_email? obj
+    return true if obj == InviteUnregisteredUserFromProposedOrg::Response::INVALID_EMAIL
+    false
+  end
+
+  def no_email? obj
+    return true if obj == InviteUnregisteredUserFromProposedOrg::Response::NO_EMAIL
+    false
+  end
+
+  def notify_registered_usr
+    NotifyRegisteredUserFromProposedOrg.new(@usr, @org).run
+    Response.new(Response::NOTIFICATION_SENT, nil, @org)
+  end
+
+  def response_obj type, message
+    Response.new(type, message, @org)
+  end
+
+  def response_type rslt_of_inviting
+    return Response::INVITATION_SENT if rslt_of_inviting.success?
+    return Response::INVALID_EMAIL if invalid_email?(rslt_of_inviting.status)
+    return Response::NO_EMAIL if no_email?(rslt_of_inviting.status)
+    Response::OTHER_ERROR
+  end
+
+  def rollback_changes_to_org
+    @org = @org.rollback_acceptance
+    @usr.organisation = nil if @usr
+    @result.not_accepted_org = @result.accepted_org
+    @result.accepted_org = nil
+    @result
+  end
+
+  def success? status
+    return true if [Response::NOTIFICATION_SENT, 
+                    Response::INVITATION_SENT].include?(status)
+    false
   end
-end
+end

app/services/create_flash_for_proposed_organisation.rb

@@ -0,0 +1,64 @@
+class CreateFlashForProposedOrganisation
+  ACCEPTED = {
+    invited: AcceptProposedOrganisation::Response::INVITATION_SENT,
+    notified: AcceptProposedOrganisation::Response::NOTIFICATION_SENT
+  }.freeze
+
+  NOT_ACCEPTED = {
+    invalid_email: AcceptProposedOrganisation::Response::INVALID_EMAIL,
+    no_email: AcceptProposedOrganisation::Response::NO_EMAIL
+  }.freeze
+
+  def initialize obj
+    @obj = obj
+    @rsl = {}
+  end
+
+  def accepted?
+    return true if [ ACCEPTED[:invited], 
+                     ACCEPTED[:notified] ].include?(@obj.status)
+    false
+  end
+
+  def flashes_for_accepted_org
+    @rsl[:notice] = ['You have approved the following organisation']
+    if_invitation_or_notification_sent
+    @rsl
+  end
+
+  def flash_for_not_accepted_org
+    @rsl[:error] = "No mail was sent because: #{@obj.error_msg}"
+    if_invalid_email
+    if_no_email
+    @rsl
+  end
+
+  def if_invalid_email
+    @rsl[:error] = 'No invitation email was sent because the email ' \
+                   "associated with #{@obj.not_accepted_org.name}," \
+                   " #{@obj.not_accepted_org.email}, seems" \
+                   ' invalid' if @obj.status == NOT_ACCEPTED[:invalid_email]
+  end
+
+  def if_invitation_or_notification_sent
+    email_type = sent_email_type
+    @rsl[:notice] << "#{email_type} was sent to " \
+                     "#{@obj.accepted_org.email}" unless email_type.empty?
+  end
+
+  def if_no_email
+    @rsl[:error] = 'No invitation email was sent' \
+                   ' because no email is associated with the' \
+                   ' organisation' if @obj.status == NOT_ACCEPTED[:no_email]
+  end
+
+  def run
+    accepted? ? flashes_for_accepted_org : flash_for_not_accepted_org
+  end
+
+  def sent_email_type
+    return 'A notification of acceptance' if @obj.status == ACCEPTED[:notified]
+    return 'An invitation email' if @obj.status == ACCEPTED[:invited]
+    ''
+  end
+end

app/services/invite_unregistered_user_from_proposed_org.rb

@@ -6,11 +6,11 @@ class Response
     OTHER_FAILURE = "Other Failure"
     SUCCESS = "Success"
 
-    attr_reader :status, :error_message
+    attr_reader :status, :error_msg
 
     def initialize(status, error_msg)
       @status = status
-      @error_message = error_msg
+      @error_msg = error_msg
     end
 
     def success?
@@ -55,6 +55,4 @@ def create_response_object(usr)
     end
     return Response.new(status,usr.errors.full_messages.first)
   end
-
-
 end

app/views/layouts/_flash.html.erb

@@ -0,0 +1,8 @@
+<div class="row">
+  <div class="span8 offset2">
+    <div id="flash_<%= "#{name}" %>" class="alert <%= bootstrap_class_for(name)%>">
+      <a class="close" data-dismiss="alert">&#215;</a>
+      <%= content_tag :div, msg %>
+    </div>
+  </div>
+</div>

app/views/layouts/application.html.erb

@@ -26,14 +26,9 @@
   <section id="main">   <!-- uniquify the child container to create the top margin -->
       <div class="container">  <!-- outermost twitter bootstrap container-->
         <% flash.each do |name, msg| %>
-            <div clas="row">
-              <div class="col-md-8 col-md-offset-2">
-                <div id="flash_<%= "#{name}" %>" class="alert alert-warning <%= bootstrap_class_for(name)%>">
-                  <a class="close" data-dismiss="alert">&#215;</a>
-                  <%= content_tag :div, msg %>
-                </div>
-              </div>
-            </div>
+          <% Array(msg).each do |item| %>
+            <%= render partial: 'layouts/flash', locals: { name: name, msg: item } %>
+          <% end %>
         <% end %>
         <div id="content">
           <%= yield %>
@@ -43,10 +38,12 @@
       <br>
       <div id="push"></div>
   </section>
+</div>
+
+<div id="footer" class="container">
+  <hr>
+  <div class="row">
 
-  <div id="footer" class="container">
-    <hr>
-    <div class="row"">
     <div class="col-md-5">
       <%= render 'layouts/footer' %>
     </div>
@@ -58,7 +55,6 @@
     </div>
   </div>
 </div>
-</div>
 
 </body>
 </html>

features/admin/admin_moderate_proposed_organisations.feature

@@ -38,14 +38,21 @@ Feature: Admin moderates an organisation to be added to HarrowCN
   Scenario Outline: superadmin accepting proposed organisations
     Given I accept a proposed organisation called "Unfriendly" with email "<email>"
     Then "<email>" is <action> as an organisation admin of "Unfriendly"
-    And I should see an error flash: "<message>"
+    And I should see "<message>"
     Examples:
-      | email                         | action   | message                                                                                       |
-      | registered_user-1@example.com | notified | A notification of acceptance was sent to registered_user-1@example.com                        |
-      | unregistered@friendly.xx      | invited  | An invitation email was sent to unregistered@friendly.xx                                      |
-      | xyt                           | ignored  | No invitation email was sent because the email associated with Unfriendly, xyt, seems invalid |
-      |                               | ignored  | No invitation email was sent because no email is associated with the organisation             |
+    | email                         | action   | message                                                                                       |
+    | registered_user-1@example.com | notified | A notification of acceptance was sent to registered_user-1@example.com                        |
+    | unregistered@friendly.xx      | invited  | An invitation email was sent to unregistered@friendly.xx                                      |
 
+  @vcr
+  Scenario Outline: superadmin cannot accept proposed organisations without a proper email
+    Given I try to accept a proposed organisation called "Unfriendly" with email "<email>"
+    Then I should see an error flash: "<message>"
+    Examples:
+    | email                         | message                                                                                       |
+    | xyt                           | No invitation email was sent because the email associated with Unfriendly, xyt, seems invalid |
+    |                               | No invitation email was sent because no email is associated with the organisation             |
+
   @vcr
   Scenario: Superadmin rejects new organisation
     And I visit the proposed organisation show page for the proposed organisation that was proposed

features/individual_charity_pages/charity_page.feature

@@ -36,7 +36,7 @@ Feature: Web page owned by each charity
     And I visit the show page for the organisation named "Friendly"
 
   @vcr
-  Scenario: Search for organisations on an organization individual page
+  Scenario: Search for organisations on an organisation individual page
     Given I select the "Advocacy" category from How They Help
     And I press "Submit"
     Then I should see "Unfriendly"

features/sign_in_sign_up/sign_in.feature

@@ -102,7 +102,7 @@ Scenario: Check that login/register toggle works
 Scenario: Check class of flash notice  - error
   Given I sign in as "superadmin@example.com" with password "12345"
   Then I should be on the sign in page
-  And the "flash_alert" should be "alert-warning"
+  And the "flash_alert" should be "alert-danger"
 
 Scenario: Check class of flash notice  - success
   Given I sign in as "superadmin@example.com" with password "pppppppp"

features/step_definitions/factory_steps.rb

@@ -16,6 +16,17 @@
   assert_on_organisation_show_page(name: name)
 end
 
+Given /^I try to accept a proposed organisation called "(.*?)" with email "(.*?)"$/ do |name, email|
+  require_relative '../../db/proposed_organisations/create'
+  Db::ProposedOrganisations::Create.new(
+    'name'  => name,
+    'email' => email,
+  ).perform
+  visit_proposed_organisation(name: name)
+  press_acceptance_for_proposed_organisation
+  assert_on_proposed_organisations_page
+end
+
 def expect_proposed_org_is_notified(acceptance_message, email, org)
   expect_email_exists(
     :message   => acceptance_message,