Skip to content

magento/magento2#: Declarative Install Schema. Add OnUpdate operation for constraints #24644

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

Closed

Conversation

atwixfirster
Copy link
Contributor

@atwixfirster atwixfirster commented Sep 18, 2019

Description (*)

Now we can add ON DELETE operation for constraints via db_schema.xml only.

Pull request adds the ability to add ON UPDATE operation for constraints.

Syntax:

onUpdate="<REFERENCE_OPTION>"

reference_option:
    RESTRICT | CASCADE | SET NULL | NO ACTION | SET DEFAULT

See Magento\Framework\DB\Adapter\AdapterInterface

Example of usage:

<constraint xsi:type="foreign" referenceId="SALES_ORDER_STATUS_STATE_STATUS_SALES_ORDER_STATUS_STATUS"
                    table="sales_order_status_state" column="status" referenceTable="sales_order_status"
                    referenceColumn="status" onUpdate="CASCADE" onDelete="CASCADE"/>

Fixed Issues (if relevant)

  1. magento/magento2#<issue_number>: Issue title

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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)

Thank you!

@m2-assistant
Copy link

m2-assistant bot commented Sep 18, 2019

Hi @atwixfirster. 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.3-develop instance - deploy vanilla Magento instance

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

@magento-engcom-team magento-engcom-team added Component: Setup Release Line: 2.3 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Sep 18, 2019
@atwixfirster atwixfirster changed the title magento/magento2#: Declarative Install Schema. Add OnUpdate feature for constraints magento/magento2#: Declarative Install Schema. Add OnUpdate operation for constraints Sep 18, 2019
@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Sep 19, 2019

Hi @buskamuza,
Could you review this PR?

As for me - looks good, but wanted to get your feedback was on update not added because of some potential issues?

Tests could be fixed once idea will be approved.

@ihor-sviziev ihor-sviziev self-assigned this Sep 19, 2019
*
* @return string
*/
public function getOnUpdate()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually backward incompatible change. As for me - it's good reason to add it. @buskamuza what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

@buskamuza any updates?

Copy link
Contributor

@ihor-sviziev ihor-sviziev Oct 3, 2019

Choose a reason for hiding this comment

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

Created issue in architecture repo magento/architecture#310, hope it will help speed up response

@akaplya
Copy link
Contributor

akaplya commented Oct 15, 2019

@atwixfirster @ihor-sviziev @buskamuza This feature was removed on purpose. We would like to keep entities identifiers as immutable values. ON UPDATE action means that object identifier can be changed somehow. If you don't have any objectives to this, let's close this PR.

@ihor-sviziev
Copy link
Contributor

Hi @atwixfirster,

I'm closing this PR as it's not an issue.

For more details see #24644 (comment)

@m2-assistant
Copy link

m2-assistant bot commented Oct 16, 2019

Hi @atwixfirster, 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.

@atwixfirster
Copy link
Contributor Author

Hi @atwixfirster,

I'm closing this PR as it's not an issue.

For more details see #24644 (comment)

Thank you, @ihor-sviziev !

That was a nice try from my side.

Thank you for explanation, @akaplya

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Setup Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: on hold Release Line: 2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants