SalesSequence. The sales sequence profiles associated to storeview ID 1 should have as prefix 1 after new Magento setup #31447

vlmed · 2020-12-23T17:52:40Z

Description (*)

Added logic to check store ID before create sequence table.

Set store ID as prefix If storeId === 1.

Modified class Magento\SalesSequence\Setup\SequenceCreator

Related Pull Requests

https://github.com/magento/partners-magento2ee/pull/471
https://github.com/magento/partners-magento2b2b/pull/557
magento/inventory#3293

Fixed Issues (if relevant)

Fixes Default created storeview sales sequences are missing a prefix #30863

Manual testing scenarios (*)

New magento install

Start new magento setup
The profiles associated to storeview ID 1 should have as prefix 1

Upgrade magento

Check or change prefix for profiles associated to storeview ID.
Upgrade magento;
The profiles associated to storeview ID 1 should have prefix as before upgrade

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

m2-assistant · 2020-12-23T17:52:43Z

Hi @vlmed. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

coderimus · 2021-01-04T13:17:18Z

Hello @vlmed
Thank you for the work you have done but to proceed with the PR review we need you to accept the Adobe CLA.

m2-assistant · 2021-01-04T13:41:01Z

Hi @vlmed, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

m2-assistant · 2021-01-04T13:41:08Z

Hi @vlmed. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

coderimus · 2021-01-04T13:44:14Z

@magento run all tests

coderimus

Thank you @vlmed for the fix and test coverage. 👍 I have only minor adjustments request for the method and class comments.

coderimus · 2021-01-04T19:38:40Z

app/code/Magento/SalesSequence/Test/Integration/Setup/SequenceCreatorTest.php

+use PHPUnit\Framework\TestCase;
+
+/**
+ * Testing data from the sales_sequence_profile table for default stores


Suggested change

* Testing data from the sales_sequence_profile table for default stores

* Test default created store views sales sequences prefix.

coderimus · 2021-01-04T19:44:02Z

app/code/Magento/SalesSequence/Test/Integration/Setup/SequenceCreatorTest.php

+    }
+
+    /**
+     * Default stores data provider


Suggested change

* Default stores data provider

* Default store codes data provider

coderimus · 2021-01-04T19:48:44Z

app/code/Magento/SalesSequence/Test/Integration/Setup/SequenceCreatorTest.php

+    }
+
+    /**
+     * Testing data from the sales_sequence_profile table for default stores


Suggested change

* Testing data from the sales_sequence_profile table for default stores

* Test prefix for default created store views.

coderimus · 2021-01-04T19:51:51Z

@magento run Integration Tests

coderimus · 2021-01-05T06:43:48Z

@magento run Integration Tests Functional Tests EE Functional Tests CE Functional Tests B2B Database Compare

hostep · 2021-03-04T10:58:54Z

@sivaschenko

Please retarget the pull request to 2.5-develop branch as the fix contains BIC

Sorry, but I don't agree with this statement.

If this change has been implemented as requested in #30863:

When this gets fixed, it should only be fixed for new Magento shops being setup, please don't add a db migration script to fix it for existing shops, that would be very unexpected for shop owners.

Then this is backwards compatible in my eyes, only new shops being setup will have this change, older existing shops will never get this change. Correct?

vlmed · 2021-03-04T15:12:49Z

@hostep It is affect only for new Magento installations. If you look in the \Magento\SalesSequence\Model\Builder::create()
you will see that before creating new Sequence tables and Profiles Magento checks if its already exist.

…r-default-created-storeview-sales-sequences

vlmed · 2021-03-24T14:53:36Z

@magento run all tests

… to compatibility with sequence prefix changing

vlmed · 2021-03-26T12:54:47Z

@magento run Functional Tests CE, Functional Tests EE, Functional Tests B2B

vlmed · 2021-03-31T13:25:24Z

@magento run all tests

hostep · 2021-06-28T21:54:23Z

Hi @sivaschenko: you never replied to my statement above?

Also: what's the status of this PR?

sivaschenko · 2021-07-04T17:51:59Z

Hi @hostep , even if these changes will be applied to the new stores only, that is still remaining a BIC for extensions that may rely their implementation on the current behaviour.

hostep · 2021-07-05T06:10:27Z

Okay, thanks for the feedback, I can live with that 🙂

magento-automated-testing · 2021-09-06T15:24:53Z