Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Update to Solidus 3.0 #286

Merged
merged 13 commits into from
Apr 26, 2021
Merged

Update to Solidus 3.0 #286

merged 13 commits into from
Apr 26, 2021

Conversation

MinasMazar
Copy link
Contributor

@MinasMazar MinasMazar commented Apr 2, 2021

Let the extension work with Solidus 3.0.

Adjust code related to Spree::Address to fit the new combined first and last name behaviour; it also update some dependency and lint rules.

@MinasMazar MinasMazar force-pushed the mm/solidus-3.0 branch 5 times, most recently from c198c5b to 50934a3 Compare April 9, 2021 10:28
@MinasMazar MinasMazar force-pushed the mm/solidus-3.0 branch 4 times, most recently from c3dd393 to c09a16b Compare April 16, 2021 13:27
Flavio Auciello and others added 10 commits April 16, 2021 16:36
The minimum version is now 0.8.1 as it includes the utility method
`SolidusSupport.combined_first_and_last_name_in_address?` that is
used in order to understand whether to use the name attribute or
the old firstname and lastname.
probably this is necessary with rspec-rails 4.x
On recent Solidus version the `::Spree::Preferences::Persistable`
module must be explicitly included in order to use preferences.
Use the already existent `SolidusPaypalbraintree::Address` service
class as a wrapper around `Spree::Address` which is responsible to
respond to both firstname/lastname and name methods, in order to be
compatible with all Solidus versions.

- fix factories
- check for other fields rather than full_name/name on expectations, in
order to work on all Solidus versions.

Co-authored-by:     Andrea Longhi <andrea@spaghetticode.it>
Co-authored-by:     Flavio Auciello <flavioauciello@nebulab.it>
We're still supporting Solidus 2.9 and 2.10, but since 2.11 we can
rely on the presence of `Spree::Address::Name` for name manipulations.
The method does not depend on the instance, plus this way it can
be easily reused elsewhere.
This seems a better place for defining this method as it deals
with the address model name.
According to [Braintree 3D Secure doc][1] the 'region' parameter now
doesn't accept state's name anymore, but abbreviation.

[1]: https://developers.braintreepayments.com/guides/3d-secure/client-side/javascript/v3#using-3ds

Co-authored-by:     Andrea Longhi <andrea@spaghetticode.it>
Co-authored-by:     Flavio Auciello <flavioauciello@nebulab.it>
Co-authored-by:     Andrea Longhi <andrea@spaghetticode.it>
Co-authored-by:     Flavio Auciello <flavioauciello@nebulab.it>
@MinasMazar MinasMazar marked this pull request as ready for review April 19, 2021 05:54
Ruby 2.4 is not supported by Rubocop since 1.12, and Solidus
is currently supporting Ruby >= 2.5.
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a question, thanks!

address.name = name
if SolidusSupport.combined_first_and_last_name_in_address?
address.name = begin
if first_name.nil?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this check? In Solidus 2.11 we'll have both first_name and name always synced and this is probably always false. In Solidus 3, there's no reason to check first_name, which is no more used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here first_name is an instance attribute of SolidusPaypalBraintree::TransactionAddress (together with last_name and name). If I recall correctly these values are returned by Braintree itself and maybe they can be valorized differently according to the API version (that's why we need the check on its presence).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah makes sense!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants