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

feat: update configuration doc #1672

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: update configuration doc #1672

wants to merge 1 commit into from

Conversation

sdo-1A
Copy link
Contributor

@sdo-1A sdo-1A commented Apr 16, 2024

Proposed change

Update documentation on configuration

@sdo-1A sdo-1A requested a review from a team as a code owner April 16, 2024 14:48
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 16, 2024
@@ -43,18 +48,19 @@ For an up-to-date documentation, run `ng help @o3r/components:extractor`
},
```

__Note:__ This options will not search for the duplicate configurations in libraries.
> [!NOTE]
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds a bit more like a warning

Suggested change
> [!NOTE]
> [!WARNING]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1,18 +1,23 @@
# Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is missing documentation regarding the O3rConfig decorator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sdo-1A
Copy link
Contributor Author

sdo-1A commented Apr 17, 2024

I did not realize that there were already updates on this documentation in another PR: #1583
So I have merged both modifications into another commit (which I will stage once we are ready to merge).

I also tried to resolve the comments from the other PR (which I will copy paste into the code to make it simpler to review).
Other comments not resolved:
https://github.com/AmadeusITGroup/otter/pull/1583/files#r1557503693
https://github.com/AmadeusITGroup/otter/pull/1583/files#r1557507384

form fields from an Angular material input element ...)
- A common configuration is defined in every library. The application common configuration (__global config__) will be
the result of the __merge of all common__ configurations.
- The __common configuration__ is the one used in multiple components. It can be a date format, a price display, an input
Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment from @cpaulve-1A:
I would put emphasis on the "common configuration" via bold text and put the inside of the parenthesis in a separate.
The type of input is really unclear and would leave out this example if we don't make it cleare. (I finally got the idea that it might be the presentation of the angular inputs)

https://github.com/AmadeusITGroup/otter/pull/1583/files#r1555909792

__Pre-bootstrap configuration__

- Defined in one interface extending the __AppBuildConfiguration__ from __@o3r/core__ in order to be identified by the extractor.
- Used for configurations needed before loading the Angular application component. They will be
Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment from @pginoux-1A:
It's not injected by the CMS, there is no runtime interactions with the CMS, it's used as an edition panel for the customization and the files are exported in the dynamic content folder afterwards. We should advertise that this part is linked to a piece of code server side that updates the index.html and what requires to be added with an example

https://github.com/AmadeusITGroup/otter/pull/1583/files#r1557804902

@sdo-1A sdo-1A force-pushed the doc/config branch 2 times, most recently from a3dca3d to 87d0b96 Compare April 18, 2024 09:14
mrednic-1A
mrednic-1A previously approved these changes Apr 18, 2024
cpaulve-1A
cpaulve-1A previously approved these changes Apr 18, 2024

- Each __component type__ can have a customized config (compliant with the store model). This custom config can be
- Each __configurable component__ can have a customized configuration (compliant with the store model). This custom configuration can be
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 a bit strange to phrase it this way and call it store configuration in the rest of the documentation.
Maybe we could call it runtime configuration and only mention the store as a way to store this configuration instead of making it seems like the update of the store is the purpose of the configuration (not just the way the configuration is stored in the application).

@sdo-1A sdo-1A force-pushed the doc/config branch 2 times, most recently from e88256f to 5c020fc Compare April 24, 2024 14:14
* If the __component extractor__ is run on an application with components from an Otter library, the `libraries` option can be used to concatenate the metadata files generated for the application with the ones from the specified libraries.
The extractor will search for the component configuration and style metadata files in the `node_modules` package of each configured library.
The name of the metadata files to search for is defined for each library in the `cmsMetadata` property defined in their respective `package.json` files.
* Here is an example of an `angular.json` file:
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks weird to have an example of angular.json after a whole paragraph about package.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you suggest rephrasing or to remove the example?

- the second priority is the priority by component type set in the store
- the lowest priority is the default config set on component (in the config.ts file of the component)
- The highest priority is the one passed as input from a parent component
- The second priority is the priority by component ID set dynamically
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure to understand this part
also, is it not already kind of covered by the schema in the Component configuration types section ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This corresponds to the customized configuration, I can rephrase if the sentence is not clear enough
And yes, this is copied in the key takeaways section of the documentation

To extract the configuration metadata from an application/library the _component extractor_ will be used. It extracts the list of all the config interfaces defined in a library or application. The output is a json file `component.config.metadata.json` containing an array of all the components configurations (app configs, pages, components).
It will also generate the component classes and types metadata from an Otter library/application, outputting them in a json file `component.class.metadata.json`.
The component extractor is used to extract the configuration metadata from an application or a library. It extracts the list of all the config interfaces defined
in a library or an application. The output is a JSON file `component.config.metadata.json` containing an array of all the component configurations (app configurations, pages, components).
Copy link
Contributor

Choose a reason for hiding this comment

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

component.config.metadata.json is the default name, it can be changed

To extract the configuration metadata from an application/library the _component extractor_ will be used. It extracts the list of all the config interfaces defined in a library or application. The output is a json file `component.config.metadata.json` containing an array of all the components configurations (app configs, pages, components).
It will also generate the component classes and types metadata from an Otter library/application, outputting them in a json file `component.class.metadata.json`.
The component extractor is used to extract the configuration metadata from an application or a library. It extracts the list of all the config interfaces defined
in a library or an application. The output is a JSON file `component.config.metadata.json` containing an array of all the component configurations (app configurations, pages, components).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure to understand the parentheses content.
Is part of the "Component Configurations"? but in this case, not sure we should indicate "app configurations". Or is the different types of configurations? If so a link to describe the different type of configs can help and maybe it would be more readable written in that way: app, pages and components configurations without being between parentheses

docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved
```
// in angular.json
* If the __component extractor__ is run on an application with components from an Otter library, the `libraries` option can be used to concatenate the metadata files generated for the application with the ones from the specified libraries.
The extractor will search for the component configuration and style metadata files in the `node_modules` package of each configured library.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not explicitly speak about node_modules, we should prefer dependencies to indicate the support of non-NPM clients

docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved

```

store config
dynamic config
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure the term is correct as they are all "dynamic config".
Maybe we can use "customized configuration"?

But a link scribing the different configuration process (or giving an example of it) can help :S

};
```

### Component file (*.component.ts)
> [!WARNING]
> Default configuration needs to be written AFTER the interface declaration and contain no variable references.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> Default configuration needs to be written AFTER the interface declaration and contain no variable references.
> Default configuration const needs to be explicitly typed with the configuration interface and contain no variable references.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can be a good practice to add a list of relative packages (link to their readme.md) in the end of the documentation to facilitate the navigation

@sdo-1A sdo-1A force-pushed the doc/config branch 2 times, most recently from 1aba2ee to d3eecd0 Compare May 14, 2024 13:49

> __WARNING__ the field name 'id' should not be used in the configuration, as we created a unique one for the entity configuration store
> [!WARNING]
> The field name `id` should not be used in the configuration, as we use this field in our internal implementation.
Copy link
Contributor

Choose a reason for hiding this comment

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

You may create a linter rule for that ! :)


- At __library__ level we have the config for components (__blocks, components, elements__).
- At __library__ level, we have the configuration for `Block` and `ExposedComponent`, see [available component types](https://github.com/AmadeusITGroup/otter/blob/main/docs/components/INTRODUCTION.md#component-type).
Copy link
Contributor

Choose a reason for hiding this comment

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

You can define component in your application also

injected by the CMS in the index.html body tag data-bootstrapconfig as a data attribute
- Defined in one interface extending the __AppBuildConfiguration__ from __@o3r/core__ in order to be identified by the extractor.
- Used for configurations needed before loading the Angular application component. It is up to the server to specify the body tag
`data-dynamiccontentpath` as a data attribute inside the `index.html` so the application can get the latest version of the dynamic
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure dynamiccontentpath as something related to the configuration

1) inline definition (see above `myUnionTypeField`)
2) reference to a union type that is defined in the same configuration file (see above `myReferencedUnionTypeField` and `Position`).

### Configuration tags
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add that we expose a linter for that docs/linter/eslint-plugin/rules/o3r-categories-tags.md

Copy link
Contributor

Choose a reason for hiding this comment

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

And a vscode extension for autocomplete

}
```

### Configuration categories
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add that we expose a linter for that docs/linter/eslint-plugin/rules/o3r-widget-tags.md

Copy link
Contributor

Choose a reason for hiding this comment

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

And a vscode extension for autocomplete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request project:@o3r/configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants