Merge pull request #381 from decko-commons/subcards

tweaks to subcard handling

card/VERSION

@@ -1 +1 @@
-0.8.0
+0.8.1

card/lib/card/act_manager.rb

@@ -76,9 +76,11 @@ class Card
   #    everything will rollback. If the integration phase fails the db changes
   #    of the other two phases will remain persistent.
   class ActManager
-    cattr_accessor :act, :act_card
+    extend EventDelay
 
     class << self
+      attr_accessor :act, :act_card
+
       def act_director
         return unless act_card
 
@@ -105,11 +107,22 @@ def clear
         self.act_card = nil
         self.act = nil
         directors.each_pair do |card, _dir|
+          card.expire
           card.director = nil
         end
+        expire
         @directors = nil
       end
 
+      def expire
+        expirees.each { |expiree| Card.expire expiree }
+        @expirees = []
+      end
+
+      def expirees
+        @expirees ||= []
+      end
+
       # FIXME: use "parent" instead of opts (it's the only option)
       def fetch card, opts={}
         return directors[card] if directors[card]
@@ -166,54 +179,11 @@ def deep_delete director
       end
 
       def running_act?
-        (dir = act_director) && dir.running?
-      end
-
-      # If active jobs (and hence the integrate_with_delay events) don't run
-      # in a background process then Card::Env.deserialize! decouples the
-      # controller's params hash and the Card::Env's params hash with the
-      # effect that params changes in the CardController get lost
-      # (a crucial example are success params that are processed in
-      # CardController#soft_redirect)
-      def contextualize_delayed_event act_id, card, env, auth
-        if delaying?
-          contextualize_for_delay(act_id, card, env, auth) { yield }
-        else
-          yield
-        end
-      end
-
-      def delaying?
-        const_defined?("Delayed") &&
-          Delayed::Worker.delay_jobs &&
-          Card.config.active_job.queue_adapter == :delayed_job
-      end
-
-      # The whole ActManager setup is gone once we reach a integrate with delay
-      # event processed by ActiveJob.
-      # This is the improvised resetup to get subcards working.
-      def contextualize_for_delay act_id, card, env, auth, &block
-        self.act = Act.find act_id if act_id
-        with_env_and_auth env, auth do
-          return yield unless act
-
-          run_act(act.card || card) do
-            act_card.director.run_delayed_event act, &block
-          end
-        end
-      end
-
-      def with_env_and_auth env, auth
-        Card::Auth.with auth do
-          Card::Env.with env do
-            yield
-          end
-        end
+        act_director&.running?
       end
 
       def to_s
         act_director.to_s
-        # directors.values.map(&:to_s).join "\n"
       end
     end
   end

card/lib/card/act_manager/event_delay.rb

@@ -0,0 +1,48 @@
+class Card
+  class ActManager
+    # methods for handling delayed events
+    module EventDelay
+      # If active jobs (and hence the integrate_with_delay events) don't run
+      # in a background process then Card::Env.deserialize! decouples the
+      # controller's params hash and the Card::Env's params hash with the
+      # effect that params changes in the CardController get lost
+      # (a crucial example are success params that are processed in
+      # CardController#soft_redirect)
+      def contextualize_delayed_event act_id, card, env, auth
+        if delaying?
+          contextualize_for_delay(act_id, card, env, auth) { yield }
+        else
+          yield
+        end
+      end
+
+      def delaying?
+        const_defined?("Delayed") &&
+          Delayed::Worker.delay_jobs &&
+          Card.config.active_job.queue_adapter == :delayed_job
+      end
+
+      # The whole ActManager setup is gone once we reach a integrate with delay
+      # event processed by ActiveJob.
+      # This is the improvised resetup to get subcards working.
+      def contextualize_for_delay act_id, card, env, auth, &block
+        self.act = Act.find act_id if act_id
+        with_env_and_auth env, auth do
+          return yield unless act
+
+          run_act(act.card || card) do
+            act_card.director.run_delayed_event act, &block
+          end
+        end
+      end
+
+      def with_env_and_auth env, auth
+        Card::Auth.with auth do
+          Card::Env.with env do
+            yield
+          end
+        end
+      end
+    end
+  end
+end

card/lib/card/act_manager/stage_director.rb

@@ -10,8 +10,8 @@ def restore_changes_information
 
   def clean_after_stage_fail
     @action = nil
-    expire_pieces
-    subcards.each(&:expire_pieces)
+    # expire_pieces
+    # subcards.each(&:expire_pieces)
   end
 
   class ActManager

card/lib/card/act_manager/stage_director/phases.rb

@@ -6,19 +6,16 @@ def validation_phase
           run_single_stage :initialize
           run_single_stage :prepare_to_validate
           run_single_stage :validate
-          @card.expire_pieces if @card.errors.any?
+        ensure
+          # @card.expire_pieces if @card.errors.any?
           @card.errors.empty?
         end
 
         def storage_phase &block
           catch_up_to_stage :prepare_to_store
           run_single_stage :store, &block
           run_single_stage :finalize
-          if @card.errors.any?
-            @card.expire_pieces
-            raise ActiveRecord::Rollback,
-                  "errors added in storage phase: #{@card.errors.full_messages * ','}"
-          end
+          raise ActiveRecord::RecordInvalid, @card if @card.errors.any?
         ensure
           @from_trash = nil
         end

card/lib/card/set/event/delayed_event.rb

@@ -41,6 +41,8 @@ def perform act_id, card, card_attribs, env, auth, method_name
             ActManager.contextualize_delayed_event act_id, card, env, auth do
               card.send method_name
             end
+          ensure
+            ActManager.expire
           end
         end
       end

card/mod/account/set/right/account.rb

@@ -64,9 +64,7 @@
 end
 
 event :send_reset_password_token do
-  Auth.as_bot do
-    token_card.update! content: generate_token
-  end
+  reset_token
   Card[:password_reset_email].deliver self, to: email
 end
 
@@ -87,18 +85,18 @@ def pending?
 end
 
 def validate_token! test_token
-  tcard = token_card
-  tcard.validate! test_token
-  copy_errors tcard
-  errors.empty?
+  token_card.validate! test_token
 end
 
 def reset_password_with_token token
   aborting do
     if !token
       errors.add :token, "is required"
     elsif !validate_token!(token)
-      # FIXME: isn't this an error??
+      # FIXME: This should be an error.
+      # However, an error abort will trigger a rollback, so the
+      # token reset won't work.  That may be an argument for
+      # handling the token update in a separate request?
       success << reset_password_try_again
     else
       success << reset_password_success
@@ -123,17 +121,15 @@ def ok_to_read
 end
 
 def reset_password_success
-  token_card.used!
+  # token_card.used!
   Auth.signin left_id
-  { id: name,
-    view: :edit }
+  { id: name, view: :edit }
 end
 
 def reset_password_try_again
+  message = tr :sorry_email_reset, error_msg: token_card.errors.first.last
   send_reset_password_token
-  { id: "_self",
-    view: "message",
-    message: tr(:sorry_email_reset, error_msg: errors.first.last) }
+  { id: "_self", view: "message", message: message }
 end
 
 # FIXME: explain or remove.

card/mod/account/set/right/token.rb

@@ -20,6 +20,7 @@ def validate! token
     when content != token then [:incorrect_token, tr(:error_incorrect_token)]
     end
   errors.add *error if error
+  error.nil?
 end
 
 def expired?

card/mod/account/set/type/signup.rb

@@ -115,10 +115,8 @@ def default_account_status
   # necessary because this performs actions as Wagn Bot
   abort :failure, "no account associated with #{name}" unless account
 
-  account.validate_token! @env_token
-
-  if account.errors.empty?
-    account.token_card.used!
+  if account.validate_token! @env_token
+    # account.token_card.used!
     activate_account
     Auth.signin id
     Auth.as_bot # use admin permissions for rest of action

card/mod/account/spec/set/right/account_spec.rb

@@ -149,7 +149,6 @@
       trigger_reset
       expect(Card::Auth.current_id).to eq(@account.left_id)
       @account = @account.refresh true
-      # expect(@account.fetch(trait: :token)).to be_nil
     end
 
     it "does not work if token is expired" do
@@ -169,7 +168,7 @@
     it "does not work if token is wrong" do
       Card::Env.params[:token] = @token + "xxx"
       trigger_reset
-      expect(@account.errors[:incorrect_token].first).to match(/mismatch/)
+      expect(Card::Env.success[:message]).to match(/mismatch/)
     end
   end
 end

card/mod/account/spec/set/type/signup_spec.rb

@@ -103,7 +103,8 @@
       # token gets updated
       expect(@account.token).not_to eq(@token)
       # user notified of expired token
-      expect(Card::Env.success.message).to match(/expired/)
+      expect(Card::Env.success.message)
+        .to match(/Please check your email for a new password reset link\./)
     end
   end
 

card/mod/core/set/all/event_conditions.rb

@@ -6,6 +6,10 @@ def event_applies? event
   end
 end
 
+def skip_event! *events
+  forced_skip_events.merge events
+end
+
 private
 
 def set_condition_applies? set_module, old_sets
@@ -68,9 +72,9 @@ def when_condition_applies? _event, block
 end
 
 def skip_condition_applies? event, allowed
-  return true if skip_events.empty?
-  event = event.name.to_s
-  !(standard_skip_event?(event, allowed) || force_skip_event?(event))
+  return true if never_skip?
+
+  !(standard_skip_event?(event.name, allowed) || force_skip_event?(event.name))
 end
 
 def trigger_condition_applies? event, required
@@ -103,17 +107,24 @@ def wrong_action action
   "on: #{action} method #{method} called on #{@action}"
 end
 
+def never_skip?
+  return @never_skip unless @never_skip.nil?
+
+  @never_skip = skip_events.empty? && forced_skip_events.empty?
+end
+
 def standard_skip_event? event, allowed
   return false unless allowed == :allowed
-  skip_events.include? event
+  skip_events.include? event.to_s
 end
 
 def force_skip_event? event
   forced_skip_events.include? event
 end
 
+# holder for skip_event! (with bang) events
 def forced_skip_events
-  @forced_skip_events ||= ::Set.new(skip_events.find_all { |e| e.last == "!" })
+  @forced_skip_events ||= ::Set.new([])
 end
 
 def skip_events

card/mod/core/set/all/name_events.rb

@@ -14,6 +14,7 @@
 event :validate_name, :validate, on: :save, changed: :name do
   validate_legality_of_name
   return if errors.any?
+  Card.write_to_soft_cache self
   validate_uniqueness_of_name
 end
 
@@ -50,8 +51,8 @@
 
 # STAGE: store
 
-event :set_name, :store, changed: :name do
-  expire
+event :expire_old_name, :store, changed: :name, on: :update do
+  ActManager.expirees << name_before_act
 end
 
 event :set_left_and_right, :store,

card/mod/core/set/all/references.rb

@@ -166,9 +166,9 @@ def each_reference_out
       on: :update, after: :name_change_finalized, when: :update_referers do
   referers.each do |card|
     next if card.structure
-    card.skip_in_action = %i[validate_renaming check_permissions!]
-    attach_subcard card.name,
-                   content: card.replace_reference_syntax(name_before_last_save, name)
+    card.skip_event! :validate_renaming, :check_permissions
+    card.content = card.replace_reference_syntax name_before_last_save, name
+    attach_subcard card
   end
 end
 

card/mod/core/set/all/rename.rb

@@ -1,4 +1,4 @@
-event :rename_in_trash, after: :set_name, on: :update do
+event :rename_in_trash, after: :expire_old_name, on: :update do
   existing_card = Card.find_by_key_and_trash name.key, true
   return if !existing_card || existing_card == self
   existing_card.name = existing_card.name + "*trash"
@@ -28,7 +28,8 @@ def suspend_name name
     newname = child.name.swap name_before_last_save, name
     # not sure if this is still needed since we attach the children as subcards
     # (it used to be resolved right here without adding subcards)
-    Card.expire child.name
+    ActManager.expirees << child.name
+    child.skip_event! :check_permissions
 
     # superleft has to be the first argument. Otherwise the call of `name=` in
     # `assign_attributes` can cause problems because `left` doesn't find the new left.