Skip to content

webui: introduce confirmation dialog on rerun and cancel job actions#951

Merged
pstorz merged 5 commits intomasterfrom
dev/fbergkemper/master/s4967
Oct 21, 2021
Merged

webui: introduce confirmation dialog on rerun and cancel job actions#951
pstorz merged 5 commits intomasterfrom
dev/fbergkemper/master/s4967

Conversation

@frb121
Copy link
Contributor

@frb121 frb121 commented Oct 7, 2021

  • introduce confirmation dialog before rerun/cancel command execution on jobs
  • jump to queued job id details view after rerun confirmation
  • stay in current view if rerun/cancel job is not confirmed

Thank you for contributing to the Bareos Project!

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • bareos-check-sources --since-merge does not report any problems
  • git status should not report modifications in the source tree after building and testing

@frb121 frb121 self-assigned this Oct 7, 2021
@frb121 frb121 force-pushed the dev/fbergkemper/master/s4967 branch from 352033b to c0141fa Compare October 7, 2021 08:36
@frb121 frb121 requested a review from pstorz October 7, 2021 08:37
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

The "Dustbin" icon is not very good for cancelling the job. I would expect a red x or something like that, as the dustbin is associated with "deleting" somethin, not cancelling.

Other than that: good work!

@frb121 frb121 force-pushed the dev/fbergkemper/master/s4967 branch from c0141fa to c041741 Compare October 18, 2021 10:32
@frb121
Copy link
Contributor Author

frb121 commented Oct 18, 2021

The "Dustbin" icon is not very good for cancelling the job. I would expect a red x or something like that, as the dustbin is associated with "deleting" somethin, not cancelling.

I agree, also it should be consistent in every place of the interface and we already used the glyphicon-remove (x) for a job cancel action on the dashboard. I changed it from glyphicon-trash (trashbin) to glyphicon-remove (x) in a separate commit.

@frb121 frb121 requested a review from pstorz October 18, 2021 10:45
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work, thank you!

@pstorz pstorz self-requested a review October 19, 2021 11:59
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Sorry, I was a bit too fast. Looking at jenkins and running the webui tests myself, it looks like some tests are now failing:

    208 - webui:admin-rerun_job (Failed)
    209 - webui:admin-restore (Failed)
    213 - webui:readonly-rerun_job (Failed)

208: selenium.common.exceptions.UnexpectedAlertPresentException: Alert Text: None
208: Message: unexpected alert open: {Alert text : Rerun Job ID 1?}

@frb121
Copy link
Contributor Author

frb121 commented Oct 19, 2021

Dang! Of course there are some adjustments required so the selenium tests can handle the changes. I'll adjust that so the tests work again.

This commit extends the test_rerun_job method to accept the newly
introduced confirmation dialog on rerun job action.
@frb121 frb121 force-pushed the dev/fbergkemper/master/s4967 branch from fa1229d to 3ffa44e Compare October 20, 2021 13:14
@frb121 frb121 requested a review from pstorz October 20, 2021 15:14
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work, the tests now pass.

@pstorz pstorz merged commit 0a7e896 into master Oct 21, 2021
@pstorz pstorz deleted the dev/fbergkemper/master/s4967 branch October 21, 2021 10:06
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