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.

Sign up for GitHub

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

Proposal for configuration provisioning #370

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+238 −0
Open

Proposal for configuration provisioning #370

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
571907c
Added proposal for configuration provisioning
sydekumf Apr 21, 2020
03e4347
Updated proposal with further information and detailed examples
sydekumf Jul 3, 2020
f73f220
Editorial pass
dobooth Jul 9, 2020
c130e1d
Updated document in regards of review
sydekumf Jul 30, 2020
c6e634f
Merge branch 'master' of github.com:sydekumf/architecture
sydekumf Jul 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 169 additions & 4 deletions design-documents/provisioning/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ The core module provides:

#### Provisioning sequence

There are entities which can be declared on project scope and entities which can be declared on module scope. All project declarations belong into `app/etc`. Basically everything which can be declared on module scope can also be declared on project scope and will be overwritten by the project scope as it enforces the rules project wide and also can change module introduced entities.

**Project declarations:**

* Attribute Sets/Attribute Groups
* Attributes
* Websites/Stores/Store Groups
* System configuration (basically the config.xml functionality will be used for this)
* Tax
* future entities can be added...

**Module declarations:**

* Attributes
* System configuration (basically the config.xml functionality will be used for this)
* Tax

The usual behavior for installing from scratch or upgrading is the following:

`bin/magento setup:install`
Expand All @@ -64,26 +81,174 @@ The usual behavior for installing from scratch or upgrading is the following:
* Recurring Data
* app:config:import

The configuration provisioning core logic should be executed as one of the last recurring data scripts. As alternative the execution sequence mentioned above can be changed so that it runs directly after recurring data and before config import.
There is a dependency tree during installation like the following:

* Declarative Schema
* Schema Patches
* Recurring Schema Scripts
* Websites => Store Groups => Stores
* System Configuration
* Attributes => Attribute Sets / Attribute Groups
* Tax
* Data Patches
* Recurring Data Scripts
* app:config:import

The reason for this is that also registered themes (inside a recurring data script) can still be used by the provisioning logic. This would be obsolete if theme registration is part of configuration provisioning.

#### Data structure

As already mentioned the data structure is related to the existing database declarative schema. Every module can define such a XML file for a configuration type. All configuration files for the same type would be merged to one big logical structure. Therefore every configuration type module should at least support merge logic. This is done individually as for example websites are merged differently than attributes.

Therefore a virtual type of `Magento\Framework\Config\Reader\Filesystem` will be defined which has a ` Magento\Framework\Config\ConverterInterface` which is responsible for merging the XML files.

On top of that every configuration type module defines a `maintained` flag which basically defines if the configuration can be overriden by a user or not. If it is set to `yes` the configuration provisioning will always override the values, independently if it was changed or not.
There is also a field `is_mutable` which defines if the declared data can be changed by the admin. The following explains the behavior:

* `is_mutable` is not defined:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to have the checkbox described more clearly, for somebody w/o context it's not very clear which checkbox is mentioned. As I understand, it's a feature similar to current "system" value? Maybe just move this "is_mutable" flag is checkbox at the attribute grid and at attribute edit page higher in the document, before "checkbox" is mentioned?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are 3 options for is_mutable values: true, false, locked. Which one corresponds to "not defined"?

* new entity is installed
* checkbox is unchecked
* admin can change the entity (after checking the checkbox)
* won't overwrite admin side manual created entities until checkbox is unchecked
* `is_mutable = false`:
* new entity is installed
* checkbox does not exist
* admin can't change the entity
* will overwrite admin side manual created entities

#### Example: Attribute behavior
- "**is_mutable**" flag is checkbox at the attribute grid and at attribute edit page
- Attributes which created from Admin panel is not maintainable by xml (no dumping, "**is_mutable**" flag is not shown but has true value in database) until xml with the same attribute code will be created and "**is_mutable**" will be unchecked
- "**is_mutable**" flag is responsible for defining maintenance responsibilities:
- **true** responsibility on Admin panel side switchable from admin
Copy link
Contributor

Choose a reason for hiding this comment

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

Why true and false options are needed? Why would we need for admin to check/uncheck the checkbox?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd reiterate on description for "is_mutable". It looks a little bit confusing right now, from different sentences.
Would it even make sense to divide it into two options?

  1. is_mutable (true, false) - if true, admin can modify the value (which will override what's declared in the file), if false, admin can't modify it
  2. revert to system value - a button/checkbox for mutable attributes in Admin UI to revert to system value (declared in the file)

Case of removal should be also covered. Should it be possible for admin to remove a mutable entity/attribute? In this case, should there be UI for restoring it from the system value?

- **false** responsibility on code side switchable from admin
- **locked** - responsibility on code side and not switchable from admin
```
<?xml version="1.0"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<attributes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:Attributes/Declaration/Data/etc/attributes.xsd">
<customer>
<attribute xsi:type="date"
code="custom_design_from"
frontend_label="Active From"
backend_type="datetime"
is_required="false"
is_user_defined="true"
is_unique="false"
backend_model="Magento\Catalog\Model\Attribute\Backend\Startdate"
attribute_model="Magento\Catalog\Model\ResourceModel\Eav\Attribute"
is_mutable="false" <!-- is optional and has default value false-->/>
</customer>
</attributes>
```
## Created by XML
- Default `"is_mutable"=false` and this property is optional for attribute xml node
- Fully managed by admin panel
- If xml with the same attribute code will be created - it will take over all responsibilities to a code side and will be then managed like attribute created by XML declaration

| is_mutable in XML-> | `is_mutable` not defined | `is_mutable="false"` |
| ------------------------- | --------------------------------- | -------------------- |
| DB value for `is_mutable` | true/false (default false) | locked |
| checkbox | unchecked | not existing |
| checkbox changeable | true | not existing |
| save btn | disabled (until checkbox checked) | disabled |
| delete btn | disabled (until checkbox checked) | disabled |
| data patch changes | can apply | will be overwritten |


## Created by admin (xml declaration doesn't exists):
- Fully managed by admin panel
- If xml with the same attribute code will be created - it will take over all responsibilities to a code side and will be then managed like attribute created by XML declaration
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be tru only for non-mutable entities? Though you may provide more real-world scenarios to understand what would be more convenient.
For me described behavior makes things more complex. Like, if I'm looking at an instance (installed by somebody some time ago), how do I know if it's a feature or a bug that some values are taken from files, and others are not (probably Admin entered them). I would prefer to not rely on time for the application logic as it's difficult to reason about.


* DB value for `is_mutable`: true
* checkbox: hidden
* checkbox changeable: false
* save btn: enabled
* delete btn: enabled
* data patch changes: can apply

#### Example: Store Scopes behavior
We can define a `stores.xml` file on project scope like the following:

```xml
<?xml version="1.0"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<store_scopes xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:Stores/Declaration/Data/etc/store_scopes.xsd">
<websites>
<website code="admin"
name="Admin"
sort_order="0"
default_group_code="default"
is_default="0"
is_mutable="false"/>
<website code="base"
name="Main Website"
sort_order="0"
default_group_code="main_website_store"
is_default="1"/>
</websites>
<store_groups>
<store_group code="default"
name="Default"
root_category_path="Root Catalog"
default_store_code="admin"
is_mutable="false"/>
<store_group code="main_website_store"
name="Main Website Store"
root_category_path="Default Category"
default_store_code="default"/>
</store_groups>
<stores>
<store code="admin"
website_code="admin"
group_code="default"
name="Admin"
sort_order="0"
is_active="1"
is_mutable="false"/>
<store code="default"
website_code="base"
group_code="main_website_store"
name="Default Store View"
sort_order="0"
is_active="1"/>
</stores>
</store_scopes>
```

#### Change or delete configuration

Changing or deleting configuration is a challenging task, especially for all the relations and true identifier of an entity (i.e. store code vs. autoincrement id). Every configuration type module should therefore define the identifier on its own, dependent on which configuration type it supports.

Changes can then be done via the identifier and the `maintained` flag.
Example: Create and update store:
- The store/website/store group is identified by its code
- All other data than the code can be changed => will lead to an update
- If the code has to change: delete entity and create new one

Deletions must be done explicit similar how you explicitly deactivate a plugin with `disabled = true`. This allows for proper merging of the XML files.

Example Delete store:
- A `deleted=true` flag should be provided
```xml
<store code="default"
website_code="base"
group_code="main_website_store"
name="Default Store View"
sort_order="0"
is_active="1"
deleted="true"/>
```

#### Extension Points and Scenarios

Once the core provisioning logic and module is set up including the main workflow, we can adapt legacy code to the new functionality. For example registering themes is done via a recurring patch which we can easily adapt to the new configuration provisioning logic.
Expand Down