Fix RedundantPresenceValidationOnBelongs on Spree::Order model

[cyrillefr]

What? Why?

• Contributes to Fix Rubocop Rails issues #11482
• First attempt was made by Fix cop RedundantPresenceValidationOnBelongs #12380 ,
• Continuation of chore(deps-dev): bump rubocop from 1.60.2 to 1.62.1 #12259
• See also Require belongs_to associations by default #11297 that started the work

This is the last file to be checked.

What should we test?

Run the tests. They should all passes

What did I do what I have done ?

No migration, no checks here, contrary to the last PRs.
My goal was to ensure some of the presence of foreign ids was done, even though they appeared optional.
But, by doing tests, it appears they should be called optional.
This is what I found:

• by making order_cycle_id, distributor_id & customer_id mandatory, I can not even pass the first page, because:

ActiveRecord::RecordInvalid in HomeController#index
Validation failed: Order cycle must exist, Distributor must exist, Customer must exist

File: File: lib/spree/core/controller_helpers/order.rb
Line:

@current_order = Spree::Order.new(currency: current_currency) <- New order almost naked
@current_order.user ||= spree_current_user
# See https://github.com/spree/spree/issues/3346 for reasons why this line is here
@current_order.created_by ||= spree_current_user
@current_order.save! <- Crashes here

Regarding created_by & user_id:

• In spec: (File spec/models/spree/order_spec.rb)

context "#associate_user!" do
  it "should associate a user with a persisted order" do
    order = create(:order_with_line_items, created_by: nil) <- Here
    # Why test with a nil ? Must have been some reason there

• Also in code (File: app/controllers/spree/admin/orders/customer_details_controller.rb)

def update
  if @order.update(order_params)
    if params[:guest_checkout] == "false"
      @order.associate_user!(Spree::User.find_by(email: @order.email)) <- Here

• Another one (File app/services/orders/cart_reset_service.rb

order.associate_user!(current_user) if order.user.blank? || order.email.blank?

The associate_user tells me that creating an order before assigning it to a user is commonplace. Same with created_by.
There are other occurrences of this in the code.

To me, the app needs these fields to be not null.
It might be a bit unusual, these fields that should be required as they are foreign keys, but that describes how the app works.
But, we can also leave the file as it is and add a line in the .rubocop_to_do not to touche it :)

Release notes

Changelog Category (reviewers may add a label for the release notes):

• User facing changes
• API changes (V0, V1, DFC or Webhook)
• Technical changes only
• Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

[cyrillefr]

Hello @sigmundpetersen ,

there is redis failed check by a too many request.
Is it possible to rerun or is there some kind of limit here ?

[sigmundpetersen]

Re-run was green 👍

[mkllnk]

Great.

The order model is used for the cart as well. And you can have a cart without even logging in. So it's totally valid to not have these associations.

[rioug]

Good one ! Thanks @cyrillefr