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

Specify cycle in eui analysis #3820

Merged
merged 15 commits into from
Mar 22, 2023
Merged

Conversation

haneslinger
Copy link
Contributor

Any background context you want to provide?

What's this PR do?

How should this be manually tested?

What are the relevant tickets?

#3801 and #3800

Screenshots (if appropriate)

image

@haneslinger haneslinger added the Enhancement Add this label if functionality was generally improved but not a full feature or maintentance. label Feb 2, 2023
@haneslinger haneslinger self-assigned this Feb 2, 2023
Copy link
Contributor

@anchapin anchapin left a comment

Choose a reason for hiding this comment

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

Overall, it looks really good. I just had two specific comments related to the analysis card that's created.

seed/tests/test_analysis_pipelines.py Show resolved Hide resolved
self.maxDiff = None
config = {
"select_meters": "date_range",
"meter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

When I select date range I only see "select_meters = date_range" but not the start and end dates. Is that intentional? It would be good to show the start and end dates. See image below.

image

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 did not alter how the analysis are displayed, let me see if I can add it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good now. However, can we have start_date first then end_date? I tried to figure out how to change this but couldn't figure it out quickly.

@axelstudios
Copy link
Member

I fixed several issues, and added some improvements:

  • Fixed date validation console errors
  • Fixed multiple forced calendar popups when clicking Select Meter Data Range
  • Fixed gaps between configuration list items in analysis details
  • Fixed broken analysis dialog from the legacy inventory list
  • Removed cycle ids from config list
  • Used container-fluid for proper layout
  • Moved Create Analysis button to the dialog actions

Thanks to @kflemin for the improved card layout:
All
Cycle
DateRange

@axelstudios axelstudios merged commit 40a685d into develop Mar 22, 2023
@axelstudios axelstudios deleted the Specify-cycle-in-eui-analysis branch March 22, 2023 00:22
@axelstudios axelstudios self-requested a review March 22, 2023 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Add this label if functionality was generally improved but not a full feature or maintentance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants