Accurately reflect pairing status on cached levels #44462
Conversation
jamescodeorg
force-pushed
the
jamescodeorg/cached-level-pairing
branch
2 times, most recently
from
aa4d416
to
e5a9bfb
Compare
jamescodeorg
force-pushed
the
jamescodeorg/cached-level-pairing
branch
2 times, most recently
from
e6348eb
to
50ec929
Compare
jamescodeorg
force-pushed
the
jamescodeorg/cached-level-pairing
branch
from
50ec929
to
56e2081
Compare
| local_assigns[:level] : | ||
| false | ||
| # Determine if we want to show the create menu | ||
| if local_assigns[:show_create_menu] |
There was a problem hiding this comment.
Do you need to check defined? here? (defined?(local_assigns))
There was a problem hiding this comment.
My understanding is that local_assigns is a method (https://apidock.com/rails/v5.0.0.1/ActionView/Template/local_assigns) so we don't need to check if it's defined? (I think the original code wasn't checking what it looked like it was checking for.)
shared/haml/user_header.haml
Outdated
| # We want to show the Create button for all pages except when working on a | ||
| # course level. Specifically, for any page except non-project levels. | ||
| should_show_create_menu = !level || level.try(:is_project_level) | ||
| level = defined?(local_assigns) ? |
There was a problem hiding this comment.
Do you need to keep this? Is level used anywhere?
There was a problem hiding this comment.
No, I don't need this anymore, nice catch!
| -# by the client. | ||
| - if @public_caching | ||
| :javascript | ||
| dashboard.header.buildUserMenu() |
There was a problem hiding this comment.
I forget, is this also called in pegasus? For pegasus cached pages is the pairing status right?
There was a problem hiding this comment.
No, this is dashboard only. The pairing status for pegasus may still be wrong. (The main goal of this fix was to fix this issue for the levels and script overview pages, but it does fix it for all of dashboard.)
This change reworks and extends the changes in PR #43716 so that the user menu on the levels page for cached levels is populated from the existing
/api/user_menuendpoint. This fixes the issue where cached levels did not properly show pairing status.This is intended to be a short-term fix to support this scenario and will be revisited when we separate pegasus from dashboard. This fix is not an ideal long-term fix because of the following:
/api/user_menuendpoint returns a rendered user_menu in html. Ideally, we would probably have an api return the data needed, put that data in redux and have react components that connected to that data. This would require rewriting the existing haml-based rendering of the user menu to react. There are also some additional complexities related to how this code is shared between pegasus and dashboard which will hopefully be clarified as we separate pegasus and dashboard./api/user_menuendpoint currently returns both the create menu and the sign-in button / user menu. At the point where we call the api, we don't have the level information needed to determine whether the create menu should be shown. As a hacky-ish workaround, we plumb through a flag that allows this code path to maintain the current state of the create menu. A better solution would be to separate the create menu from the sign-in button / user menu but that was a bit more complicated, again due to how pegasus and dashboard share code here.Screen.Recording.2022-01-24.at.7.06.21.PM.mov
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: