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

Add rule template support #1173

Merged
merged 3 commits into from
Oct 3, 2021
Merged

Add rule template support #1173

merged 3 commits into from
Oct 3, 2021

Conversation

ghys
Copy link
Member

@ghys ghys commented Oct 1, 2021

Closes #1082.

Signed-off-by: Yannick Schaus github@schaus.net

Signed-off-by: Yannick Schaus <github@schaus.net>
@relativeci
Copy link

relativeci bot commented Oct 1, 2021

Job #219: Bundle Size — 10.65MB (~+0.01%).

ae7452e vs 8ff4240

Changed metrics (1/8)
Metric Current Baseline
Cache Invalidation 1.24% 1.22%
Changed assets by type (1/7)
            Current     Baseline
JS 8.38MB (~+0.01%) 8.38MB

View Job #219 report on app.relative-ci.com

@ghys
Copy link
Member Author

ghys commented Oct 1, 2021

@kaikreuzer can you please answer this as I'm not 100% positive I got it right:

  • do the rule templates only assist in creating the rule, and then the modules can be freely edited (and in that case, why save the templateUID and configuration as part of the rule?),

  • or can the configuration be altered after the rule has been created, and the modules will be refreshed with the new values in placeholders (in that case, the UI should hide the modules and only display the parameters as described by the template when editing too?).

ghys added 2 commits October 1, 2021 17:44
Signed-off-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Yannick Schaus <github@schaus.net>
@kaikreuzer
Copy link
Member

do the rule templates only assist in creating the rule, and then the modules can be freely edited

Yes, correct.

why save the templateUID and configuration as part of the rule?
can the configuration be altered after the rule has been created

The template UID is imho indeed not necessary.
The configuration is meant to enable configurations on rule level that are then automatically considered by the modules. It might well be that this is technically not yet fully implemented, so maybe better ignore these configurations in the UI for now.

If this is what you have implemented so far and you think it should be directly included in M3, please remove the WIP and I'll wait with launching the M3 build until the merge of this PR.

@ghys
Copy link
Member Author

ghys commented Oct 3, 2021

Yes it's what was implemented. There's the little issue we experienced yesterday that the template parameters are not checked when the rule is created (like all required parameters have been filled in) and the template might not be applied if there are errors.
Other than that I believe it works well overall, we can probably add the parameter checks in a follow-up PR. It would be nice to open the marketplace to rule templates early to accept contributions and iron out the eventual bugs then.
If you agree then I'll merge this.

@ghys ghys added enhancement New feature or request main ui Main UI labels Oct 3, 2021
@ghys ghys added this to the 3.2 milestone Oct 3, 2021
@ghys ghys changed the title WIP: Add rule template support Add rule template support Oct 3, 2021
@ghys ghys marked this pull request as ready for review October 3, 2021 12:27
@ghys ghys requested a review from a team as a code owner October 3, 2021 12:27
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Alright, let's merge then - thank you!

@kaikreuzer kaikreuzer merged commit 3842515 into openhab:main Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for rule templates
2 participants