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

Autofill presenter name with the info from /me.json #1037

Merged

Conversation

narickmann
Copy link
Contributor

If the presenter field is empty (and no value in local storage) the users name from info/me.json is used.

Copy link
Contributor

@lkiesow lkiesow left a comment

Choose a reason for hiding this comment

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

If Opencast is unreachable, Studio shows an error. Once the connection can be established, the error disappears and the event can be uploaded. This code seems to cause a stack trace and seems to be unable to recover, however:

error when getting info/me Error: network error when accessing 'https://develop.opencast.org/info/me.json': TypeError: NetworkError when attempting to fetch resource.
    q https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    Z https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    request https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    jsonRequest https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    getInfoMe https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    updateUser https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    refreshConnection https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    e https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/823.1e6ee22f.chunk.js:1
    setTimeout handler*7448/Ae/< https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/823.1e6ee22f.chunk.js:1
    nl https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    Su https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    ku https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    S https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    A https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    6813 https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    __webpack_require__ https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    5296 https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    __webpack_require__ https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    4463 https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    __webpack_require__ https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    4164 https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    __webpack_require__ https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    <anonymous> https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    <anonymous> https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
    <anonymous> https://test.studio.opencast.org/2023-05-11_08-04-44-pr1037-4945454464/static/js/main.ce2c9eee.js:2
main.ce2c9eee.js:2:7963

This also seems to cause studio to load /info/me.json over and over again, as fast as it can at least when the user does not actually have a name. Deploying this could therefor actually lead to an unwanted DOS attack:

studio-auto-fill-name.mp4

At least the latter error should be addressed

@github-actions github-actions bot added the status:conflicts Conflicts with another pull request or issue label Aug 24, 2023
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot removed the status:conflicts Conflicts with another pull request or issue label Aug 30, 2023
@narickmann
Copy link
Contributor Author

Studio should not load 'info/me.json' over and over again.

If Opencast is unreachable, Studio shows an error. Once the connection can be established, the error disappears and the event can be uploaded. This code seems to cause a stack trace and seems to be unable to recover

This should (hopefully) also been fixed. Not sure how to test it.

Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

I can confirm that both problems found by Lars seem to be fixed now!

Oh and your PR accidentally includes an empty save-creation/index.tsx which should be removed.

src/steps/finish/upload.tsx Outdated Show resolved Hide resolved
src/opencast.tsx Outdated Show resolved Hide resolved
This patch adds a config option to specify a preference order
of potential sources to autofill the presenter field from.
Leaving it empty is equivalent to the current behavior.
The only valid value right now is `"opencast"`,
which gets the value from `/info/me.json`.
The way the feature is built, it should be easy to extend
in the future, though.
If there is a name stored in `localStorage` already,
this currently takes precedence over (any of) the suggested value(s).

Co-authored-by: naweidner <weidner@elan-ev.de>
Copy link
Member

@LukasKalbertodt LukasKalbertodt 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 trying to get this PR done! Some comments, but just minor things.

CONFIGURATION.md Outdated Show resolved Hide resolved
CONFIGURATION.md Outdated Show resolved Hide resolved
src/opencast.tsx Outdated Show resolved Hide resolved
src/opencast.tsx Outdated Show resolved Hide resolved
src/opencast.tsx Outdated Show resolved Hide resolved
src/steps/finish/upload.tsx Outdated Show resolved Hide resolved
src/steps/finish/upload.tsx Outdated Show resolved Hide resolved
src/steps/finish/upload.tsx Outdated Show resolved Hide resolved
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Seems to work!

@LukasKalbertodt LukasKalbertodt merged commit 59bea17 into elan-ev:master Nov 16, 2023
2 checks passed
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.

4 participants