-
Notifications
You must be signed in to change notification settings - Fork 0
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
FW-4520 Use Stats API #201
Conversation
import api from 'services/api' | ||
import { useParams } from 'react-router-dom' | ||
|
||
export function useStats() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for how the stats widget displays the data from the stats API has been moved here rather than WidgetStatsData.js
const weekTotal = getPeriodTotal('last7Days') | ||
const monthTotal = getPeriodTotal('lastMonth') | ||
|
||
if (weekTotal > 4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stats widget should work the same as when the data was being provided by Nuxeo, however I have been unable to trigger the widget into displaying "new this week", "new this month" and "on this site" are working correctly though. @gmcauliffe I am not sure if this is how the widget behaved before the endpoint change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API returns "lastWeek" not "last7Days"... does this time period string need to match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was it! The display is now fixed, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just one small import change and you're good to merge :)
// FPCC | ||
import { useSiteStore } from 'context/SiteContext' | ||
import api from 'services/api' | ||
import { useStats } from '../../common/dataHooks/useStats' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using relative imports. You can leverage our Webpack aliases here and change this to 'common/dataHooks/useStats'
Kudos, SonarCloud Quality Gate passed! |
Description of Changes
This PR re-enables the stats widget and connect it to the new stats API in the Django BE.
Checklist
Reviewers Should Do The Following:
Additional Notes
N/A