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

Make the Dashboard page compatible with the new data model #1493

Merged

Conversation

roseeichelmann
Copy link
Contributor

@roseeichelmann roseeichelmann commented Jul 25, 2024

Associated issues

Closes cityofaustin/atd-data-tech#18328

The issue suggests using the crashes list view but the way this currently works is we are supposed to reflect the same values in this VZE that are in the socrata dataset, so i thought it made the most sense to use the socrata_export_crashes_view

Testing

URL to test:
Local

Steps to test:

  1. Check out this branch, start up your data model db and apply hasura migrations/metadata and run the cris import script if necessary
  2. Open up the hasura console and navigate to the socrata_export_crashes_view, sort by death count, years of life lost, and suspected serious injuries to see what we would expect the widgets on the dashboard to say. Keep in mind this is only for crashes that happened in 2024.
  3. Start your VZE, go to the dashboard, and confirm you are seeing the numbers you would expect to see in the widgets.
  4. Now go create some temp crash records with fatalities/serious injuries to further test the dashboard widget. Remember that the socrata dataset is only counting crashes that happened at least 14 days ago. So in your crash creation the date needs to be todays date - 14 days or farther back.
  5. Also, you will need to update the private drive flag to NO in the crash details page of the temp record in order for it to factor into the socrata ytd totals. Sidenote - lets not let this requirement slip through the cracks so the VZ team isnt wondering why their temp crashes arent making it into the socrata totals
  6. Update your computer to a different time zone. You should still see the same numbers in your widgets bc everything should be using UTC time and not expecting the user to be in CT.

Ship list

  • Check migrations for any conflicts with latest migrations in master branch
  • Confirm Hasura role permissions for necessary access
  • Code reviewed
  • Product manager approved

@roseeichelmann roseeichelmann added the WIP Work in progress label Jul 25, 2024
@@ -347,13 +347,13 @@ const CreateCrashRecord = ({ client }) => {
fade={false}
>
{/*eslint-disable-next-line*/}
<i className="fa fa-check-circle" /> Successfsully
created crash{" "}
<i className="fa fa-check-circle" /> Successfully created
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lil typo 😄

const yearEnd = format(subDays(new Date(), 14), "yyyy-MM-dd");
const yearEnd = subDays(new Date(), 14)
.toISOString()
.substring(0, 19); // to match the ISO string format in the socrata view crash_timestamp column
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are creating a date in UTC time so that this is cross compatible with users in any time zone. cutting off the last milliseconds in the ISO string bc the crash_timestamp column in the socrata view doesnt have milliseconds

Copy link
Member

Choose a reason for hiding this comment

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

i think you can do away yearEnd completely, since we have the 14-day cutoff baked into the socrata view 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hah you're so right wow what a silly oversight on my part 😅

@roseeichelmann roseeichelmann removed the WIP Work in progress label Jul 29, 2024
crashes(where: { is_temp_record: { _eq: true } }) {
crashes(
where: { is_temp_record: { _eq: true } }
order_by: { record_locator: asc }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was annoying me that the temp records table wasnt being sorted so it was hard to find ur last record creation

Copy link
Member

Choose a reason for hiding this comment

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

do you want to sort this by desc instead, so the most recent records are at the top?

also, i could have sworn you had configured the query to refetch after form submit so that the temp records table gets refreshed. that's not happening anymore. maybe i broke that when i patched your branch?? 🙈

Copy link
Member

@johnclary johnclary left a comment

Choose a reason for hiding this comment

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

i ran through all the tests—it works great. love that you went with the socrata export view for this!

const yearEnd = format(subDays(new Date(), 14), "yyyy-MM-dd");
const yearEnd = subDays(new Date(), 14)
.toISOString()
.substring(0, 19); // to match the ISO string format in the socrata view crash_timestamp column
Copy link
Member

Choose a reason for hiding this comment

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

i think you can do away yearEnd completely, since we have the 14-day cutoff baked into the socrata view 🤔


import { GET_CRASHES_YTD } from "../../queries/dashboard";

import bi_logo from "../../assets/img/brand/power_bi_icon_white_on_transparent.png";

function VZDashboard() {
const year = new Date().getFullYear();
const yearStart = `${year}-01-01`;
const yearStart = `${year}-01-01T00:00:00`;
Copy link
Member

Choose a reason for hiding this comment

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

little TZ bug here—to get the year value you want to use new Date().getUTCFullYear() since you're constructing a UTC timestamp

crashes(where: { is_temp_record: { _eq: true } }) {
crashes(
where: { is_temp_record: { _eq: true } }
order_by: { record_locator: asc }
Copy link
Member

Choose a reason for hiding this comment

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

do you want to sort this by desc instead, so the most recent records are at the top?

also, i could have sworn you had configured the query to refetch after form submit so that the temp records table gets refreshed. that's not happening anymore. maybe i broke that when i patched your branch?? 🙈

@chiaberry
Copy link
Member

fwiw I didnt see the temp records table refresh when i tested last week

@roseeichelmann
Copy link
Contributor Author

@johnclary ready for re-review, i dont think there was ever refetch functionality but i just added that in!

@roseeichelmann roseeichelmann requested a review from johnclary July 29, 2024 22:10
const [deleteRecord] = useMutation(DELETE_TEMP_RECORD);

useEffect(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dont need this useEffect

});
};
// /**
// * Deletes all the temporary records in the database
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just commenting this out till we handle temp deletes in a following PR

Copy link
Member

@johnclary johnclary left a comment

Choose a reason for hiding this comment

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

niceeee—thanks for adding in the refetch!

🚢 🚢 🚢 🚢 🚢 🚢 🚢

@roseeichelmann roseeichelmann merged commit b97cb93 into 18149-data-model-socrata-etls Jul 30, 2024
@roseeichelmann roseeichelmann deleted the 18328_dashboard_data_model branch July 30, 2024 18:39
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.

3 participants