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

MBS-8967: Add modal to show errors when trying to change sth #46

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

sh-csg
Copy link
Contributor

@sh-csg sh-csg commented Mar 22, 2024

Implements #14

@sh-csg sh-csg self-assigned this Mar 22, 2024
@sh-csg
Copy link
Contributor Author

sh-csg commented Mar 22, 2024

Failing GHA is not related to this PR

@sh-csg sh-csg mentioned this pull request Mar 26, 2024
Copy link
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

This fixes part of the issue when trying to do for example things you aren't allowed to.

However, the error handling should be implemented for every webservices call, also for the permanent update polls for example. Currently, if for testing purposes I insert an exception into get_kanban_content_update the app will just crash.

The error handling should properly handle the issue of connection loss.

Maybe it's also possible/easier to use displayException function?

@sh-csg
Copy link
Contributor Author

sh-csg commented May 24, 2024

Added an alert for displaying connection loss.

Copy link
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

I left a few comments. Have not tested yet.

amd/src/mutations.js Show resolved Hide resolved
amd/src/mutations.js Outdated Show resolved Hide resolved
amd/src/mutations.js Outdated Show resolved Hide resolved
amd/src/mutations.js Outdated Show resolved Hide resolved
amd/src/mutations.js Outdated Show resolved Hide resolved
amd/src/mutations.js Outdated Show resolved Hide resolved
amd/src/mutations.js Show resolved Hide resolved
amd/src/mutations.js Show resolved Hide resolved
@PhMemmel PhMemmel force-pushed the MBS-8967-error-handling branch from b44e867 to 5791dad Compare September 18, 2024 12:27
@sh-csg sh-csg force-pushed the MBS-8967-error-handling branch from 068d9ff to 5afced9 Compare December 16, 2024 06:39
Copy link
Member

@PhMemmel PhMemmel 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 well so far, good job!

Would it be difficult to implement a feedback for actions the user is doing? If he tries to create a card for example there is no feedback if this action is being queued or it's just being ignored or what happens with it.

Another solution would be to disable the UI completely until the connection is back up.

Thoughts?

@sh-csg
Copy link
Contributor Author

sh-csg commented Jan 13, 2025

Seems to work well so far, good job!

Would it be difficult to implement a feedback for actions the user is doing? If he tries to create a card for example there is no feedback if this action is being queued or it's just being ignored or what happens with it.

Another solution would be to disable the UI completely until the connection is back up.

Thoughts?

Yes

amd/src/mutations.js Outdated Show resolved Hide resolved
@PhMemmel
Copy link
Member

Would it be difficult to implement a feedback for actions the user is doing? If he tries to create a card for example there is no feedback if this action is being queued or it's just being ignored or what happens with it.
Another solution would be to disable the UI completely until the connection is back up.
Thoughts?

Yes

What a pity! Then I'll raise a separate ticket for this ;-)

@sh-csg sh-csg force-pushed the MBS-8967-error-handling branch from d8782b6 to f86fd2a Compare January 14, 2025 08:13
@PhMemmel PhMemmel force-pushed the MBS-8967-error-handling branch from f86fd2a to 7dcf7a5 Compare January 14, 2025 08:15
@PhMemmel
Copy link
Member

In theory we would have to wrap each ajax with a try catch to avoid all JS console errors. The major ones now have been wrapped, rest is fine for throwing JS console errors when there's a big red banner showing that connection has been lost :)

@PhMemmel PhMemmel merged commit 2320be0 into master Jan 14, 2025
20 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.

2 participants