Update extension for new Omnes event 🚌 #193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
forkata
changed the title
forkata
commented
Jun 24, 2022
app/subscribers/super_good/solidus_taxjar/spree/reporting_subscriber.rb
Outdated
Show resolved
Hide resolved
benjaminwil
approved these changes
Jun 27, 2022
app/subscribers/super_good/solidus_taxjar/spree/reporting_subscriber.rb
Outdated
Show resolved
Hide resolved
forkata
force-pushed
the
chore/get-on-the-new-bus
branch
from
6d7fe01 to
ccafe94
Compare
forkata
commented
Jun 27, 2022
forkata
force-pushed
the
chore/get-on-the-new-bus
branch
2 times, most recently
from
61e13a6 to
e9cb906
Compare
forkata
changed the title
forkata
requested review from
benjaminwil,
adammathys,
Noah-Silvera,
nvandoorn and
stewart
forkata
commented
Jul 15, 2022
app/overrides/super_good/solidus_taxjar/spree/order_updater/fire_recalculated_event.rb
Show resolved
Hide resolved
adammathys
approved these changes
Jul 18, 2022
Noah-Silvera
approved these changes
Jul 25, 2022
nvandoorn
approved these changes
Aug 8, 2022
|
Converting this to a draft as me and @Noah-Silvera discovered an issue with the |
forkata
force-pushed
the
chore/get-on-the-new-bus
branch
2 times, most recently
from
936116e to
25902c5
Compare
Noah-Silvera
force-pushed
the
chore/get-on-the-new-bus
branch
6 times, most recently
from
5862053 to
f0725ff
Compare
Noah-Silvera
force-pushed
the
chore/get-on-the-new-bus
branch
from
f0725ff to
1b1e171
Compare
Noah-Silvera
changed the base branch from
master
to
update-solidus-dependencies-to-support-3.2
forkata
force-pushed
the
chore/get-on-the-new-bus
branch
from
1b1e171 to
941a7fd
Compare
Base automatically changed from
update-solidus-dependencies-to-support-3.2
to
master
September 19, 2022 17:56
The specs here were specific to the legacy implementation of the event system, and therefore no longer pass on the `master` branch of Solidus where we now use a [new implementation of the event bus](solidusio/solidus#4342). This change attempts to refactor the tests to focus on observing the side-effects of the object under test which should be independent of the underlying implementation. This test is valuable regardless of what version of Solidus we are running against as our extension relies on the event being published when a specific action happens and that the payload contains the necessary object. Co-authored-by: Andrew Stewart <andrew@super.gd>
This change attempts to introduce a backwards compatible way for registering and publishing events on the new Omnes event bus introduced in Solidus. A few of the issues this change resolves are * auto-loading of subscribers is not a feature of the event bus anymore and we need to explicitly register subscribers with the event bus * firing of events in tests was done in a manner not forward compatible with the new event bus, so we now leverage the compatibility layer provided by `solidus_support` * custom events now need to be registered in an initializer and use symbol for the event name instead of strings
We are now using the compatibility layer for the event system which was introduced in 0.9.0.
This change uses the compatibility layer provided by `solidus_support` to fire the `shipment_shipped` event we have introduced in this extension in a way that works with the new `Omnes` event bus. This will ensure that this continues to work with versions of Solidus >= 3.2.0. Co-authored-by: Noah Silvera <noah@super.gd>
Documents the fact that we now require a newer version of `solidus_support` which in turn requires a newer Rails version.
Noah-Silvera
force-pushed
the
chore/get-on-the-new-bus
branch
from
941a7fd to
4de6296
Compare
|
@Noah-Silvera, @benjaminwil and I confirmed the issue with Solidus 3.2 is present upstream as well so not related to the changes merged here. I will work on manually setting up a working sandbox on that version of Solidus so we can verify the new behaviour introduced here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the goal of this PR?
Get tests passing against Solidus
master.Update the way we use the Solidus event bus to be forwards compatible with the new Omens bus implementation that is now default in Solidus "=> 3.2.0.alpha" - solidusio/solidus#4342
Additional information on updating to the new event bus implementation - solidusio/solidus#4380
How do you manually test these changes?
masterMerge Checklist