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

User Dashboard - Summary Statistics Portlet #930

Merged
merged 26 commits into from
Jul 1, 2019

Conversation

ryanrath
Copy link
Contributor

Description

This PR refactors the hard coded Summary Statistics from the Summary tab to be config file driven and contained within a Novice User Portlet.

Motivation and Context

We want to be able to display these statistics using the new Novice User interface.

Tests performed

Integration Tests were added that exercise the new summary/statistics end point. They cover:

  • Happy Path for all users
  • Pub user w/ no start date
  • Pub user w/ no end date
  • Pub user w/ no start or end date
  • Pub user w/ an invalid start date
  • Pub user w/ an invalid end date
  • Pub user w/ start date after end date

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.

jpwhite4 and others added 5 commits May 17, 2019 09:16
This adds a new portlet, `SummaryStatisticsPortlet`, to OpenXDMoD. It provides
the same functionality as the summary statistics bar at the top of the old
Summary page ( via `summary3.php` ).

- The format of the statistics have been refactored out of `summary3.php` and
into a configuration file that can be modified:
`configuration/roles.d/summary_statistics.json`.
- The portlet is made visible to users via: `configuration/roles.d/summary.json`
Added a new integration test to exercise the `summary/statistics` REST endpoint
and the associated artifacts.
The changes to `SummaryControllerProviderTest.php` were needed to account for
validating `float` data values differently then other values.

Also updated the expected error message for start_date after end_date
ryanrath added 3 commits May 20, 2019 16:13
- Removed `DeveloperPortlet.js` as it shouldn't have been committed in the first
  place.
- Moved `SummaryControllerProviderTest` & it's related test artifacts to the
  `rest` sub dir as opposed to `controllers`.
- Updated comment @ the end of `SummaryStatisticsPortlet.js` to be correct.
- re: returning 400's vs 500's.
  - Made sure that `BadRequestHttpException`'s are thrown where appropriate in
  `SummaryControllerProvider`.
Just pretty printing the test artifacts so that it's easier to see when
something changes.
Just some simple reformatting.
@ryanrath ryanrath force-pushed the novice_summary_statistics branch from 240cd5e to e409576 Compare May 21, 2019 13:58
@ryanrath ryanrath requested review from plessbd and jpwhite4 May 23, 2019 17:01
@jpwhite4 jpwhite4 added the Category:User Dashboard Screen shown after user login label May 29, 2019
@jpwhite4
Copy link
Member

The code I was thinkg of is in the checkDateParameters() part of the classes/DataWarehouse/Access/Common.php file

ryanrath added 5 commits June 13, 2019 13:30
We don't have any use cases for the `filters` parameter in this request. If we
do eventually have one, we can come back and snag this code.
Needed some additional information to help track down statistic irregularities.
@ryanrath
Copy link
Contributor Author

@jpwhite4 nod I checked out classes/DataWarehouse/Access/Common.php and I could probably shoehorn it in to work w/ the new ControllerProviders, but it wouldn't be very pretty. I do have a set of changes that remove the validation checks in TimeAggregationUnit and instead provide a validateDateRange($startDate, $endDate) function in BaseControllerProvider that takes care of making sure that both $startDate and $endDate are valid && that $startDate comes before $endDate, throwing a 400 if any of those things are not true. Thoughts?

Just added a `checkDateRange` function that will throw a 400 if either date is
not in the correct format or if the start date is after the end date. Also
removed the previously added date validation in the `TimeAggregationUnit` code.
ryanrath added 9 commits June 14, 2019 15:49
Updated the `checkDateRange` function to work like the date validation code that
was previously added to `Query` as the diff'ing was waaaay easier this way.

Also updated the expected output to account for the new error message.
Removed the unused `duration_change` listener as we no longer base our dates on
the duration toolbar.

Added documentation for the `formatDate` function.
The `getDateTime` function was no long returning a `DateTime` object but was
instead returning a unix timestamp. The function name / variables created from
this function have been renamed accordingly.
The `$tz` argument was for a previous iteration of the code so as it's not being
used it is being removed.
@ryanrath ryanrath merged commit bff4a73 into ubccr:xdmod8.5 Jul 1, 2019
@plessbd plessbd changed the title Novice User Interface - Summary Statistics Portlet User Dashboard - Summary Statistics Portlet Oct 16, 2019
@plessbd plessbd added this to the 8.5.0 milestone Oct 16, 2019
@plessbd plessbd added the new feature New functionality label Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:User Dashboard Screen shown after user login new feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants