- moved more code into models

- make sure the created_at timestamp is right in the test cases

app/models/subscription_fu/subscription.rb

@@ -3,7 +3,7 @@ class SubscriptionFu::Subscription < ActiveRecord::Base
 
   AVAILABLE_CANCEL_REASONS = %w( update cancel timeout admin )
 
-  default_scope order(:created_at, :id)
+  default_scope order("created_at ASC", "id ASC")
 
   belongs_to :subject, :polymorphic => true
   belongs_to :prev_subscription, :class_name => "SubscriptionFu::Subscription"

examples/transactions_controller.rb

@@ -23,18 +23,11 @@ def update
 
   def require_valid_transaction
     @token = params[:token]
-    @subscription = current_user.subscriptions.last
-    if @subscription.activated?
-      logger.info("Subscription is already activated")
-      flash[:notice] = "Subscription is already activated"
+    @transaction = current_group.pending_transaction(@token)
+    unless @transaction
+      logger.info("Invalid transaction for token: #{@token}")
+      flash[:error] = "Invalid transaction, please try again."
       redirect_to root_path
-    else
-      @transaction = @subscription.transactions.initiated.find_by_identifier(@token)
-      unless @transaction
-        logger.info("Invalid transaction")
-        flash[:error] = "Invalid transaction, please try again."
-        redirect_to root_path
-      end
     end
   end
 end

lib/subscription_fu/models.rb

@@ -30,6 +30,16 @@ def upcoming_subscription
         active_subscription ? active_subscription.next_subscriptions.activated.last : nil
       end
 
+      def pending_transaction(identifier)
+        sub = subscriptions.last
+        if sub.activated?
+          logger.info("Latest subscription is already activated")
+          nil
+        else
+          sub.transactions.initiated.find_by_identifier(identifier)
+        end
+      end
+
       def build_next_subscription(plan_key)
         if active_subscription
           # TODO refactor

spec/models/subscription_spec.rb

@@ -74,6 +74,9 @@ def should_have_free_activation_flow(sub_instance, first_sub)
         context "authorize" do
           before { @redirect_target = @trans.start_checkout("http://return.to", "http://cancel.to") }
           it("should redirect to return URL") { @redirect_target.should == "http://return.to" }
+          it("should be pending transaction on subject") do
+            instance_variable_get("@#{sub_instance}").subject.pending_transaction(@trans.identifier).should == @trans
+          end
           context "complete" do
             before { mock_paypal_delete_profile("fgsga564aa") } unless first_sub
             before { @trans.complete! }
@@ -96,6 +99,9 @@ def should_have_paid_activation_flow(sub_instance, first_sub, prev_sub_is_free =
           it("should redirect to return URL") do
             @redirect_target.should == "https://www.paypal.com/cgi-bin/webscr?cmd=_express-checkout&token=token123"
           end
+          it("should be pending transaction on subject") do
+            instance_variable_get("@#{sub_instance}").subject.pending_transaction("token123").should == @trans
+          end
           context "complete" do
             before { mock_paypal_create_profile("token123", "6bvsaksd9j") }
             before { mock_paypal_delete_profile("fgsga564aa") } unless first_sub
@@ -146,7 +152,7 @@ def should_have_paid_activation_flow(sub_instance, first_sub, prev_sub_is_free =
     before { @initiator = Factory(:initiator)  }
 
     context "free subscription" do
-      before { @sub = Factory(:subscription, :plan_key => "free") }
+      before { at_time("2010-01-10 00:00 UTC") { @sub = Factory(:subscription, :plan_key => "free") } }
       it("should indicate it isn't a paid subscription") { @sub.should_not be_paid_subscription }
       it("should return human name") do
         pending do
@@ -181,8 +187,10 @@ def should_have_paid_activation_flow(sub_instance, first_sub, prev_sub_is_free =
         @now = Time.parse("2010-01-12 11:45")
         @sub_start = Time.parse("2010-01-10 00:00 UTC")
         @next_billing = Time.parse("2010-02-10 00:00 UTC")
-        @sub = Factory(:subscription, :plan_key => "premium", :paypal_profile_id => "fgsga564aa",
-                       :starts_at => @sub_start, :billing_starts_at => @sub_start, :activated_at => @sub_start)
+        at_time(@sub_start) do
+          @sub = Factory(:subscription, :plan_key => "premium", :paypal_profile_id => "fgsga564aa",
+                         :starts_at => @sub_start, :billing_starts_at => @sub_start, :activated_at => @sub_start)
+        end
       end
 
       context "active on paypal" do