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

Update alexa integration with new metadata syntax #1145

Merged
merged 6 commits into from Nov 14, 2021

Conversation

jsetton
Copy link
Contributor

@jsetton jsetton commented Aug 16, 2021

Related to openhab/openhab-alexa#394.

This change should be merged after the new metadata syntax is released to the production environment.

Since the new syntax is backward compatible, users of previous GUI versions will still be able to use the old interface.

@jsetton jsetton requested a review from a team as a code owner August 16, 2021 06:49
@jsetton jsetton marked this pull request as draft August 16, 2021 17:19
@jsetton jsetton force-pushed the alexa-update branch 3 times, most recently from e1e7e7d to d3c71b9 Compare August 21, 2021 14:57
@relativeci
Copy link

relativeci bot commented Aug 25, 2021

Job #239: Bundle Size — 10.69MB (-0.09%).

0351031 vs f888fbf

Changed metrics (3/8)
Metric Current Baseline
Initial JS 1.65MB(-0.52%) 1.65MB
Cache Invalidation 25.58% 1.41%
Modules 1506(-1.25%) 1525
Changed assets by type (1/7)
            Current     Baseline
JS 8.42MB (-0.12%) 8.43MB

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

Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
Copy link
Contributor Author

jsetton commented Sep 9, 2021

@ghys could you take a look at this one when you have a chance? It's pretty much ready for review. They will be some minor data input changes based on the beta testing feedback and bug fixes. I just want to make sure that we can merge this one once we move the Alexa metadata changes to the live skill.

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.

Thanks for the huge update @jsetton - I only have a few minor comments, apart from those the code looks good, on the functionality itself I'll trust you as I don't use Alexa or follow the latest developments.

Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
Copy link
Contributor Author

jsetton commented Oct 9, 2021

@ghys sorry for the delay. I made the changes based on your review comments. Let me know what you think.

@ghys
Copy link
Member

ghys commented Oct 12, 2021

@jsetton thanks - I have no more comments.
Remove the draft status to have it merged when the backend side is ready!

Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
Copy link
Contributor Author

jsetton commented Oct 28, 2021

@ghys Not sure if you saw my last review request. I wanted to make sure you are fine with the last change I made with the items query. We are currently planning to move forward with the production release next week.

@ghys
Copy link
Member

ghys commented Oct 28, 2021

@jsetton not really, and don't see any new request?
I'm not notified on new commits (this last one f483c59 looks fine).
What is the new items query?

@jsetton
Copy link
Contributor Author

jsetton commented Oct 28, 2021

@ghys I added a comment in line of that change, although I clicked on starting a review. I am not sure if these trigger notifications though.

@ghys
Copy link
Member

ghys commented Oct 28, 2021

I don't see the comment, maybe you started a review and forgot to finish it? (it should say "pending" so you have to click on the green button on the top-right to finish the review).

@jsetton
Copy link
Contributor Author

jsetton commented Oct 28, 2021

I just switched to a single comment. Can you see it now?

Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
Copy link
Contributor Author

jsetton commented Oct 30, 2021

@ghys I decided to change how cross-device connections are defined with the network capabilities using group relationships in the end. This means that I reverted the items query to just pulling specific group items. So that should limit heavy requests.

As far as your last comment, I have moved the API requests from created to mounted Vue event. I also added a f7-preloader while this.ready is false in the event a specific item is a member of a high number of groups.

Hopefully, you are fine with the last change. This is the last functional change that I will be making on this PR.

@ghys
Copy link
Member

ghys commented Oct 31, 2021

These changes are fine by me!

Signed-off-by: jsetton <jeremy.setton@gmail.com>
Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton jsetton marked this pull request as ready for review November 10, 2021 06:52
@jsetton
Copy link
Contributor Author

jsetton commented Nov 10, 2021

@ghys This PR is ready to be merged. Relevant changes have been merged and deployed to the production environment.

@digitaldan
Copy link
Contributor

Looking forward to seeing this merged! @jsetton has our next release of the Alexa skill ready to go.

@relativeci
Copy link

relativeci bot commented Nov 14, 2021

Job #254: Bundle Size — 10.78MB (+0.31%).

84d1d6a vs 56e0f9d

Changed metrics (1/8)
Metric Current Baseline
Cache Invalidation 5.47% 0.76%
Changed assets by type (1/7)
            Current     Baseline
JS 8.51MB (+0.39%) 8.47MB

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

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.

Thank you for this extensive overhaul! - since I don't use Alexa I will trust you on the functional side, but the UI seems to work at it should, and the code lgtm.

@ghys ghys merged commit 1671c1a into openhab:main Nov 14, 2021
@ghys ghys added enhancement New feature or request main ui Main UI labels Nov 14, 2021
@ghys ghys added this to the 3.2 milestone Nov 14, 2021
@jsetton jsetton deleted the alexa-update branch November 14, 2021 16:25
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.

None yet

3 participants