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

Discipline Dashboard release #2374

Merged
merged 4 commits into from Jan 29, 2019

Conversation

Projects
None yet
2 participants
@edavidsonsawyer
Copy link
Collaborator

edavidsonsawyer commented Jan 28, 2019

Who is this PR for?

Educators

What problem does this PR fix?

The discipline dashboard is available in production, but there are no live links to the feature.

What does this PR do?

Adds links to the discipline dashboard alongside the tardies/absences dashboards

Screenshot (if adding a client-side feature)

admin_disc_links
teacher_disc_links

Checklists

Which features or pages does this PR touch?

  • Home page

Does this PR use tests to help verify we can deploy these changes quickly and confidently?

  • Included specs for changes
  • Manual testing made more sense here
@edavidsonsawyer

This comment has been minimized.

Copy link
Collaborator Author

edavidsonsawyer commented Jan 28, 2019

@kevinrobinson I have this ready to go but we probably want to hold off on this request until we have a solution for the SHS loading time. I'm working on that today.

@kevinrobinson

This comment has been minimized.

Copy link
Contributor

kevinrobinson commented Jan 28, 2019

@edavidsonsawyer Awesome! This looks good, and can we remove the experimental banner too when this is ready to go out? The other wrinkle here is that some users have additional links on the navbar - you can sign in as the jodi or harry user to see this. I think it will fit with a 1024px wide screen, but can you confirm?

re: SHS loading, I think the issue is the n+1 where it queries for all students, then for each one queries for discipline incidents. The absences page batches these queries up and might be a model to check out: https://github.com/studentinsights/studentinsights/blob/master/app/lib/dashboard_queries.rb#L8. If there are ways to make smaller changes that'd be great to try, but just using the same approach as the absences page is fine too, which might be easier without profiling.

@edavidsonsawyer

This comment has been minimized.

Copy link
Collaborator Author

edavidsonsawyer commented Jan 28, 2019

@kevinrobinson thanks for flagging the users with extra links. I tested with both of them at a bunch of resolutions and everything seems to fit. One note is that I don't think the Jodi user is supposed to have access to the dashboards (she doesn't now). Is that not correct?

@kevinrobinson

This comment has been minimized.

Copy link
Contributor

kevinrobinson commented Jan 28, 2019

edavidsonsawyer added some commits Jan 28, 2019

@edavidsonsawyer

This comment has been minimized.

Copy link
Collaborator Author

edavidsonsawyer commented Jan 28, 2019

@kevinrobinson okay, the banner is out, so this should be ready when we want to merge it.

@kevinrobinson

This comment has been minimized.

Copy link
Contributor

kevinrobinson commented Jan 29, 2019

@edavidsonsawyer 🎉 awesome, going out now!

@kevinrobinson kevinrobinson merged commit 66c8665 into studentinsights:master Jan 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment