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

Adding an alert to notify users when ingestion has not been run. #1389

Merged
merged 6 commits into from
Jul 21, 2020

Conversation

ryanrath
Copy link
Contributor

Description

Adding an informational alert for when either ingestion or acl-config has
not been run.

This is to address:
https://app.asana.com/0/342819846538629/1185258719202835

Motivation and Context

Tests performed

Manually tested.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Adding an informational alert for when either ingestion or `acl-config` has
not been run.

This is to address:
https://app.asana.com/0/342819846538629/1185258719202835
@ryanrath ryanrath changed the title Adding an alert to notify a user when ingestion has not been run. Adding an alert to notify users when ingestion has not been run. Jul 20, 2020
html/index.php Outdated
Ext.MessageBox.alert(
'Invalid State Detected',
'This XDMoD installation has had an invalid state detected. <br/><br/>' +
'Before XDMoD can be utilized you must run ingestion at least once.<br/><br/>' +
Copy link
Contributor

Choose a reason for hiding this comment

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

so does the no aggregated data message show up somewhere else? Cause I think we have to have both ingestion and aggregation done before it actually works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure to be honest, but I can update the phrasing to reference both ingestion / aggregation. The direct cause of this problem is that there are no records in moddb.realms when they try to load the front page.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried it and the one, new message, is displayed. Nothing after that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jtpalmer did you add all of the changes from the PR? I know that before I added the try-catch around the disabledMenus section the page came up empty because the exception from Acls::getDisabledMenus function ultimately ended up outputting a blob 'o json describing the exception right in the middle of those js variables.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we only have it for jobs, and only when we are not using the dashboard
https://github.com/ubccr/xdmod/blob/xdmod9.0/html/controllers/ui_data/summary3.php#L71

Wonder if we should find a way to test for it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test for what? Not having run ingestion / aggregation and / or acl-config?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ryanrath I testing using your branch (at the time that I posted the comment). I did not shred or ingest any jobs so it's possible that if I had done that, but not aggregated that something different would happen.

Copy link
Contributor

@jtpalmer jtpalmer left a comment

Choose a reason for hiding this comment

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

A couple wording suggestions.

html/index.php Outdated Show resolved Hide resolved
html/index.php Outdated Show resolved Hide resolved
Copy link
Contributor

@jtpalmer jtpalmer left a comment

Choose a reason for hiding this comment

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

I realized that this could never happen for XSEDE so you can safely change the text to use "Open XDMoD"

html/index.php Outdated Show resolved Hide resolved
html/index.php Outdated Show resolved Hide resolved
ryanrath and others added 4 commits July 20, 2020 15:09
Co-authored-by: Jeffrey T. Palmer <jeffrey.t.palmer@gmail.com>
Co-authored-by: Jeffrey T. Palmer <jeffrey.t.palmer@gmail.com>
Co-authored-by: Jeffrey T. Palmer <jeffrey.t.palmer@gmail.com>
Co-authored-by: Jeffrey T. Palmer <jeffrey.t.palmer@gmail.com>
@ryanrath ryanrath merged commit 1941fb2 into ubccr:xdmod9.0 Jul 21, 2020
@jtpalmer jtpalmer added this to the 9.0.0 milestone Aug 11, 2020
@jtpalmer jtpalmer added Category:General General enhancement Enhancement of the functionality of an existing feature labels Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:General General enhancement Enhancement of the functionality of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants