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

[Basic UI] Fix proxy calls for sitemaps created through the UI #860

Merged
merged 5 commits into from
Feb 11, 2021

Conversation

bigbasec
Copy link
Contributor

This is a change to the way basic ui looks up sitemap names for Image and Video types(and any other future types that utilize the proxy).

PR for the core.proxy service is here : openhab/openhab-core#2162

Description of issues covered by this :
https://community.openhab.org/t/images-video-sitemaps-and-proxy/115677

…nts to the proxy call

Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@bigbasec
Copy link
Contributor Author

LGTM

Just a heads up here, this won't fix the proxy part of things, it just fixes the rendering for the widgets.

openhab/openhab-core#2162 would resolve the proxy side so things actually work.

@ghys
Copy link
Member

ghys commented Jan 30, 2021

@bigbasec are there dependencies between these PR i.e. should we wait for the others to be merged first?

@bigbasec
Copy link
Contributor Author

@bigbasec are there dependencies between these PR i.e. should we wait for the others to be merged first?

This won't have any effect on a system that doesn't have the core PR, no. The only 2 widgets that use the "sitemap" variable are Image and Video, and those are also the only 2 that need the proxy.

So no, putting this in won't affect anything, but alone it won't fix the images and videos.

@bigbasec
Copy link
Contributor Author

bigbasec commented Feb 7, 2021

@bigbasec are there dependencies between these PR i.e. should we wait for the others to be merged first?

Dependencies have been merged.

@ghys ghys added the rebuild trigger a new Jenkins job label Feb 7, 2021
Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
@bigbasec
Copy link
Contributor Author

bigbasec commented Feb 8, 2021

Getting the hang of this git thing. That should resolve the build issue.

@bigbasec
Copy link
Contributor Author

Anything I need to do here to get this merged in? Although there wasn't any dependency on this from the core, this is required to get the core working again. Explained here : openhab/openhab-core#2178

@kaikreuzer kaikreuzer removed the rebuild trigger a new Jenkins job label Feb 11, 2021
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.

All fine, sorry for the delay and many thanks!

@kaikreuzer kaikreuzer merged commit 3a51cb4 into openhab:main Feb 11, 2021
@kaikreuzer kaikreuzer added the bug Something isn't working label Feb 11, 2021
@kaikreuzer kaikreuzer added this to the 3.1 milestone Feb 11, 2021
@kaikreuzer kaikreuzer changed the title Fixes sitemap and proxy calls [Basic UI] Fix proxy calls for sitemaps created through the UI Feb 11, 2021
@bigbasec bigbasec deleted the sitemap_and_proxy_changes branch February 11, 2021 12:52
kaikreuzer pushed a commit that referenced this pull request Feb 21, 2021
* Passes sitemap to the renderers for proper display and makes adjustments to the proxy call

Signed-off-by: Brian Homeyer <bhomeyer@gmail.com>
@kaikreuzer kaikreuzer added the patch A PR that has been cherry-picked to a patch release branch label Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants