Skip to content

Warning when submission parsing failed #2318

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Apr 20, 2025

When the report contains failed submissions, the browser displays an alert to the user informing them about it

Also adds the names of failed submissions to the information view

@Kr0nox Kr0nox added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Apr 20, 2025
@Kr0nox Kr0nox requested a review from a team April 20, 2025 06:05
@Kr0nox Kr0nox added this to the 6.1.0 milestone Apr 20, 2025
Copy link

// send a warning the first time the report is opened with the failed submissions
const failedSubmissionNames = json.failed_submission_names as string[]
if (failedSubmissionNames.length > 0) {
const hasSendWarning = sessionStorage.getItem('failedSubmissionWarning')
Copy link
Member

Choose a reason for hiding this comment

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

What happens if I load another report with failed parsing ? Or is this not possible anymore ? Does this reset the session store ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that could be an issue.
We could always reset the token when opening the landing page, although that might not cover some edge cases.
Alternativly we could store some (nearly) unique identifier for a report/failed submission list to store what the last report we sent the warning for is.
What do you think is the best way? @jplag/maintainer

Copy link
Member

Choose a reason for hiding this comment

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

Do I understand it correctly that this scenario would always arise when opening more than one report at the same time, where at least one submission failed? Then we definitely need a solution here. I'm not sure what the drawbacks of both approaches here are, so hard to judge from my end.

Copy link
Member

Choose a reason for hiding this comment

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

We should discuss in the next meeting(s) whether this is a one-time warning needed and what is the best way to inform users that there have been problems without making the warning a) too easy to overlook and b) too annoying. I would guess there is still more to experiment with than we can achieve until the next release. This feature is too important to rush a change that makes everything worse.

@tsaglam tsaglam requested a review from a team April 22, 2025 11:25
@robinmaisch robinmaisch removed this from the 6.1.0 milestone Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants