Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi shipping insurance compatibility #208

Merged

Conversation

Francois-Gomis
Copy link
Collaborator

Reason for change

Code based on Main branches not devellop for Zip to marketplace

Linear task

Code changes

Change observer order management in array for multi shipping.
$order -> [$order]
for multi shipping we receive [$order1, $order2]

How to test

Test multi shippping in magento 2

As a reviewer, you are encouraged to test the PR locally.

Checklist for authors and reviewers

  • The title of the PR uses business wording, not technical jargon, for the changelog readers to understand it
  • The PR implements the changes asked in the referenced task / issue
  • The automated tests are compliant with the testing strategy
  • The tests are relevant, and cover the corner/error cases, not only the happy path
  • You understand the impact of this PR on existing code/features

Non applicable

@Francois-Gomis Francois-Gomis requested review from a team as code owners October 23, 2024 15:16
@github-actions github-actions bot added the type: feature New feature label Oct 23, 2024
Copy link

sonarcloud bot commented Oct 23, 2024

Copy link
Contributor

@olance olance left a comment

Choose a reason for hiding this comment

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

LGTM 👍

You could probably refactor the setup code of your tests into a single utility function, but not a blocker IMHO

Copy link
Contributor

@remi-zuffinetti remi-zuffinetti left a comment

Choose a reason for hiding this comment

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

Approve not to block, but yes, if you could rebase next time to avoid this :D
Thanks !

@Francois-Gomis
Copy link
Collaborator Author

@olance How do you see the refactoring of the setup?

@Francois-Gomis Francois-Gomis merged commit 7104336 into develop Oct 29, 2024
3 of 4 checks passed
@Francois-Gomis Francois-Gomis deleted the feature/ecom-2180-ac-fix-multi-shipping-orders branch October 29, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants