Rename order updater methods to better reflect behaviour

[Noah-Silvera]

Summary

In preparation for more changes to the order updater in #5872, we should rename the existing methods to better reflect what they actually do. We want update_ to encapsulate behaviour that persists, and recalculate_

Even though some of these are private methods, we are aware of a non-trivial amount of applications that override the methods of this core class, so we want to deprecate this renames before fully removing the alias'

• OrderUpdater#update_shipment_state -> OrderUpdater#recalculate_shipment_state
• OrderUpdater#update_payment_state -> OrderUpdater#recalculate_payment_state
• OrderUpdater#update_payment_total -> OrderUpdater#recalculate_payment_total
• OrderUpdater#update_order_total -> OrderUpdater#recalculate_order_total
• OrderUpdater#update_shipment_total -> OrderUpdater#recalculate_shipment_total
• OrderUpdater#update_item_total -> OrderUpdater#recalculate_item_total
• OrderUpdater#update_item_totals -> OrderUpdater#recalculate_item_totals
• OrderUpdater#update_item_count -> OrderUpdater#recalculate_item_count
• OrderUpdater#recalculate_adjustments -> OrderUpdater#update_adjustments

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I agree that my PR will be published under the same license as Solidus.
• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have localized any and all user-facing strings that I added to the source code.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• 🛣️ I have opened a PR to update the guides.
• ✅ I have added automated tests to cover my changes.
• 📸 I have attached screenshots to demo visual changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.45%. Comparing base (3660447) to head (a859efc).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6368   +/-   ##
=======================================
  Coverage   89.44%   89.45%           
=======================================
  Files         972      973    +1     
  Lines       20297    20314   +17     
=======================================
+ Hits        18155    18172   +17     
  Misses       2142     2142           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jarednorman]

This all looks good to me, but I'd love some reviews from people outside Super Good.

[tvdeyen]

Thanks. The last commit message has a typo. Other than that, great work.

[tvdeyen]

the git commit message has a typo

[Noah-Silvera]

Good catch, fixed the typo!

[mamhoff]

I'm a little bit confused by the last commit. So we rename the update_* methods to recalculate_*, because they don't persist - great. But then we rename recalculate_ methods to update_*, because they also don't persist? I don't know.

The other caveat I have here is that we have no protection for people who have subclassed and overridden the renamed methods. This can be done by combining the self.inherited and self.method_added hooks as I've done here: https://github.com/solidusio/solidus/blob/main/promotions/app/models/solidus_promotions/condition.rb#L104-L133. I'm not making this a condition for merging though, just a suggestion that you can go with or not.

[forkata]

I'm a little bit confused by the last commit. So we rename the update_* methods to recalculate_*, because they don't persist - great. But then we rename recalculate_ methods to update_*, because they also don't persist? I don't know.

😂 Good callout @mamhoff, I think that's a typo in the commit message. The recalculate_adjustments method definitely persists to the database because it rebuilds the adjustments on the order, so the intention with that change is to move that to the update_ naming to reflect that. We can reword the commit title and message since it deals with a single method.

This naming change is meant to help with the intention behind the methods provided by the order updater class. We have no control what anyone outside of this codebase does in their overrides, so I don't know that we need to take that into consideration, but happy to hear your thoughts if you think these methods are often overridden in apps.

There is also a minor inconsistency here worth calling out, with the top level API of this class - OrderUpdater#recalculate, but we have already decided to move away from using Order#update and OrderUpdater#update a long time ago to prevent overloading Active Record APIs. This change is in preparation for #5872 where we found it useful to distinguish between the methods that need to write to the database, vs. ones that do not when working on an in-memory version of the OrderUpdater.