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

Jump to bottom

Add support for automatic creation of ManagedProviders for UI components #2948

Merged
merged 3 commits into from
May 5, 2022
Merged

Add support for automatic creation of ManagedProviders for UI components #2948

merged 3 commits into from
May 5, 2022

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented May 3, 2022 •
edited
Loading

Supersedes #2947
Follow-Up to #2617

See https://community.openhab.org/t/openhab-3-3-milestone-discussion/132715/98

Every registry needs a managed provider for storing configurations on server-side.

Signed-off-by: Jan N. Klug github@klug.nrw

@J-N-K J-N-K requested a review from a team as a code owner May 3, 2022 15:20
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label May 3, 2022
@J-N-K
Copy link
Member Author

J-N-K commented May 3, 2022

@kaikreuzer I guess this is more in line with what we do with the other registries and does not require a component for each namespace.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-3-milestone-discussion/132715/115

@J-N-K
cleanup
ba3cad4 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K force-pushed the bug-fixmanageduiprovidersnotavailable branch from ba2946c to ba3cad4 Compare May 3, 2022 17:41
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-3-milestone-discussion/132715/119

ghys
ghys reviewed May 4, 2022
View reviewed changes
Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

IMHO this should be probably be merged ASAP if it effectively fixes the regressions of the incomplete refactoring that unfortunately made it to M4 and broke several important features.

...enhab.core.ui/src/main/java/org/openhab/core/ui/internal/components/UIComponentProvider.java Outdated Show resolved Hide resolved
@ghys
Copy link
Member

ghys commented May 4, 2022 •
edited
Loading

Another remark: to be complete, other objects allowing both managed and non-managed providers have a editable flag in their REST API schema in order to give clues to UIs which ones can be saved using the API and which ones are effectively read-only, so they can adapt the display (🔒 icons & hidden functionality).
This has also been omitted in the recent refactoring so UIs can't make these adaptations for UI components that cannot be modified or deleted since they're not provided from the ManagedProvider.

FWIW that's part of the reason there wasn't any support for non-managed Providers in the initial implementation (I'm not questioning the added value of having them): there wasn't an initial need for them since they were supposed to be managed by WYSIWYG UI editors only.

I would argue it's not that urgent right now to address this because there aren't any non-managed UIComponentProviders in the official distribution yet.

@J-N-K
address review comment
a84abbe 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K force-pushed the bug-fixmanageduiprovidersnotavailable branch from 3edfac0 to a84abbe Compare May 4, 2022 19:42
kaikreuzer
kaikreuzer approved these changes May 5, 2022
View reviewed changes
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.

Great solution, many thanks!

@kaikreuzer kaikreuzer merged commit ad3a084 into openhab:main May 5, 2022
@kaikreuzer kaikreuzer added this to the 3.3 milestone May 5, 2022
@kaikreuzer
Copy link
Member

@J-N-K & @ghys Do you consider this a critical fix, which would warrant doing an M5 release out-of-schedule the next days?

@J-N-K J-N-K deleted the bug-fixmanageduiprovidersnotavailable branch May 5, 2022 07:43
@J-N-K
Copy link
Member Author

J-N-K commented May 5, 2022 •
edited
Loading

Probably, since it affects quite a few people. I would suggest to merge #2945 (not critical, but easy to review and potentially irritating behavior) and possibly #2949 (reported on the forum, regression) before that.

@ghys
Copy link
Member

ghys commented May 5, 2022

I agree there's a relatively strong case for that. In M4 users can't see or save:

  1. UI-designed sitemaps
  2. HABPanel configuration
  3. Block libraries (for Blockly rules).

@kaikreuzer
Copy link
Member

Ok, I'll suggest we test a bit tomorrow and I'll see to do a new milestone build on Saturday then.

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-3-3-milestone-discussion/132715/121

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
@J-N-K @splatch
Add support for automatic creation of ManagedProviders for UI compone…
c4f7080 
…nts (openhab#2948)

* Add support for automatic creation of ManagedProviders for UI components

Signed-off-by: Jan N. Klug <github@klug.nrw>
GitOrigin-RevId: ad3a084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghys ghys ghys left review comments

@kaikreuzer kaikreuzer kaikreuzer approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.
4 participants
@J-N-K @openhab-bot @ghys @kaikreuzer