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

fix missing hub uuid error #60

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Conversation

matthewhilton
Copy link

The issue

  • A few months ago there was an update to the library submodule that added some error messages, however, the HUB registration error - Missing Site UUID or Hub Secret. Please check your Hub registration message was incorrect and would show even if the hub was not enabled.
  • See https://h5ptechnology.atlassian.net/browse/HFP-3883
  • This was fixed in a newer version of the library submodule

The fix

  • Updated the library submodule to this version. Since this is a fork, I tried to update as little as possible to avoid any regressions to only updated to the exact hash needed.
  • Comparing the diff, it seems to very minor things e.g. fonts. If these do end up causing regressions we can easily roll it back and handle it another way.

Testing
To reproduce the bug you can do:

  • Ensure the hub is not enabled / not used.
  • View the admin settings any page, this just triggers h5p to query the hub info and queue the error in your session.
  • View any h5p, this will then dequeue these errors all at once.

After updating the library submodule, the errors are no longer shown.

@matthewhilton
Copy link
Author

Note CI is failing due to pre-existing issues, checking the CI, the tests are passing which is what really matters here.

Copy link

@keevan keevan left a comment

Choose a reason for hiding this comment

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

All the changes seem sane and relatively safe. This one (h5p/h5p-php-library@3e1f903) is mildly interesting but I'm assuming the flexible handling for importing it is already there.

Going to mark this one as okay overall since it's from upstream.

@keevan keevan merged commit 0148599 into stable-catalyst-400 Sep 6, 2024
7 of 24 checks passed
@keevan keevan deleted the fix-hub-uuid-error branch September 6, 2024 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants