Different shipment numbers after spree 2.0.7 upgrade

[alepore]

Since the upgrade from spree 2.0.6 to 2.0.7 i see shipment number changed from "H(11 digits)" to "(9 digits)"

I think after this commit 1b51211 a permalink is generated by

make_permalink field: :number

before the call of

before_create :generate_shipment_number

Before, instead, to_param was called first of all, i guess, generating the correct number before the permalink.

I think we have to rollback to the old behavior (make_permalink is really needed?)

[peterberkenbosch]

the old to_param was doing it totally wrong so that's what fixed in the commit. Should be calling the same methods now, but only by setting the proper callbacks on the model.

I'm surprised that the number of digits is changed though.

[alepore]

number of digits is changed because the method that now generates shipment number is https://github.com/spree/spree/blob/2-0-stable/core/lib/spree/core/permalinks.rb#L46 instead of https://github.com/spree/spree/blob/2-0-stable/core/app/models/spree/shipment.rb#L255

[peterberkenbosch]

I see! the before_filter here get's overridden by the make_permalink method here since the before_validation callback comes first.

[radar]

We could make this overridable (just like permalink_prefix) by adding a new option to make_permalink

[radar]

Sorry about that @alepore! I've added a fix for this to 2-0-stable, 2-1-stable and master now, along with a regression test. Permalinks can now have different lengths.

[alepore]

Thanks!
Just a question.. i see shipment number was generated by generate_shipment_number before (with the leading H) before and is now (always) generated from the generate_permalink, without the H. Is this intended?

[radar]

No, that's another bug. Will look tomorrow

On Mon, Dec 9, 2013 at 6:24 PM, Alessandro Lepore
notifications@github.com wrote:

Thanks!

Just a question.. i see shipment number was generated by generate_shipment_number before (with the leading H) before and is now (always) generated from the generate_permalink, without the H. Is this intended?

Reply to this email directly or view it on GitHub:
#4063 (comment)

[alepore]

Ok thanks, my thought was to simply remove the make_permalink field: :number line from shipment.rb and always use generate_shipment_number (as before), but maybe i'm missing something with that permalink stuff...

[radar]

@alepore I am unable to reproduce that issue on a 2-1-stable store or 2-0-stable store. Here's what I get:

irb(main):001:0> shipment = Spree::Shipment.new
=> #<Spree::Shipment id: nil, tracking: nil, number: nil, cost: nil, shipped_at: nil, order_id: nil, address_id: nil, state: "pending", created_at: nil, updated_at: nil, stock_location_id: nil>
irb(main):002:0> shipment.send(:generate_shipment_number)
  Spree::Shipment Load (0.2ms)  SELECT "spree_shipments".* FROM "spree_shipments" WHERE "spree_shipments"."number" = 'H55661247402' LIMIT 1
=> "H55661247402"

[alepore]

@radar you are manually calling generate_shipment_number, but i think that code is actually not used during shipment creation because the permalink callback comes first:

(clean spree 2-0-stable with sample data)

>> shipment = Spree::Shipment.new(order: Spree::Order.last)
  Spree::Order Load (0.4ms)  SELECT "spree_orders".* FROM "spree_orders" ORDER BY "spree_orders"."id" DESC LIMIT 1
=> #<Spree::Shipment id: nil, tracking: nil, number: nil, cost: nil, shipped_at: nil, order_id: 3, address_id: nil, state: "pending", created_at: nil, updated_at: nil, stock_location_id: nil>
>> shipment.save
   (0.3ms)  begin transaction
   (0.3ms)  SELECT COUNT(*) FROM "spree_shipments" WHERE (spree_shipments.number LIKE '27328805273%')
...

this is with 2-0-stable @ 076a3f4

>> shipment = Spree::Shipment.new(order: Spree::Order.last)
  Spree::Order Load (0.2ms)  SELECT "spree_orders".* FROM "spree_orders" ORDER BY "spree_orders"."id" DESC LIMIT 1
=> #<Spree::Shipment id: nil, tracking: nil, number: nil, cost: nil, shipped_at: nil, order_id: 3, address_id: nil, state: "pending", created_at: nil, updated_at: nil, stock_location_id: nil>
>> shipment.save
   (0.3ms)  begin transaction
  Spree::Shipment Load (0.3ms)  SELECT "spree_shipments".* FROM "spree_shipments" WHERE "spree_shipments"."number" = 'H72276651745' LIMIT 1
...

my point is: we are now generating shipment number with generate_permalink instead of generate_shipment_number, as in pre-2.0.7 versions, so we don't have the H before the numbers.

[radar]

Wow, I am stupid. Sorry @alepore, and thank you for being patient with me.

I've now got a proper fix in to my 2-0-stable, 2-1-stable and master which should fix this. Please check it and see if this does fix it.

[alepore]

👍 thanks!