Implement the levelbuilder version of extra links for Lab2 #58410
Conversation
| const ExtraLinks: React.FunctionComponent<ExtraLinksProps> = ({ | ||
| levelId, | ||
| }: ExtraLinksProps) => { | ||
| const {loading, data} = useFetch('/api/v1/users/current/permissions'); |
There was a problem hiding this comment.
I had us get all permissions here because we eventually want to extend this to more than just levelbuilders, and it would be useful to know what permissions a user has here.
| # Get the extra links for the level, for use by levelbuilders. | ||
| # This is used by lab2 levels that cannot use the haml "extra links" box | ||
| # as that box will not refresh when changing levels. | ||
| def extra_links |
There was a problem hiding this comment.
I don't know if this is where this logic should all live long term. It might make more sense to return a slimmed down version that the frontend can parse. In order to get something working for levelbuilders I think it's a decent start (I was able to copy a lot of this from the haml file), but I am tracking improving it here
| @@ -78,7 +78,8 @@ | |||
| -# special tracking image to make sure a cookie is set when hoc starts (/hoc/1) | |||
| = image_tag(tracking_pixel_url(@script)) | |||
|
|
|||
| = render partial: 'levels/admin' | |||
| - if current_user && (!@level.uses_lab2? || (@level.uses_lab2? && !current_user.levelbuilder?)) | |||
There was a problem hiding this comment.
Just to confirm - since previously there are no checks here for current_user or current_user.levelbuilder, are we okay to add them here in addition to the lab2 check? Logically seems fine to me, but just want to double check if there are any other assumptions changing here (e.g. if there's another permission type that's able to view the extra links box)
There was a problem hiding this comment.
Yeah it should be fine. The first line of the haml file is
if current_user && (current_user.levelbuilder? || current_user.project_validator? || Experiment.get_editor_experiment(current_user))
So we were already gating on current_user. Then we either show the box if this isn't a lab2 level (same as before), or if it is lab2 and the user is not a levelbuilder (so we can show the links for project validators).
| }, | ||
| } | ||
| ); | ||
| if (response.ok) { |
There was a problem hiding this comment.
nit: I've typically used both response.ok and try/catch together, and now I'm wondering why. I think it's to catch errors related to network issues or CORS (not included in response.ok?) and any errors that don't happen until the JSON parsing step. Thoughts? Maybe it's not necessary when I do it.
There was a problem hiding this comment.
I'm still getting used to fetch vs ajax, adding a try/catch makes sense!
There was a problem hiding this comment.
FWIW, HttpClient functions should throw even when a response is not ok, so just wrapping those in a try/catch should be sufficient!
The gray extra links box used by levelbuilders and project validators does not work correctly on lab2 levels because it is populated via haml. Therefore, if the level changes and we don't reload the page (which is the case for switching between lab2 levels within a lesson), the box won't get updated. This creates a react version of the extra links box. To keep things scoped I just implemented the levelbuilder version of this (and skipped a few things for now), and left the project validator version in haml. See follow-up work for details on that.
Instead of a box that can hide/show, there is now a button the pops a modal up.
To get this to work, I made a new level api
/extra_links, which returns the links for a level. There was a strong desire from curriculum to get a minimal version working asap, so I skipped a few things, specifically on the level side listing contained levels and a lot of blockly links that aren't currently used in any lab2 levels. Long-term this api can be improved/slimmed down to not be so prescriptive as to what we show on the frontend, but it was easiest for now to get parity with haml to put this on the rails side.In order to get delete to work, I needed to update the delete api to check the accept header and return a json response if we ask for it. Otherwise it was trying to do a redirect the client could not process.
The project validator version of this box will need data generate from elsewhere (or may be mostly be able to be generated from the frontend), as it is not level-specific but project specific. Since projects are always loaded from the server it made sense to split this work out into a separate task and continue using the old haml version for now.
Before
After
New Modal
Python Lab
Clone button clicked
Delete button clicked
Note I left start and exemplar in the modal as start mode is actively being worked on, and exemplars are a near-term task.
Music Lab
AI Chat
Links
Testing story
Tested on Music Lab, Python Lab, and AI Chat levels. This button only shows up for levelbuilders, otherwise we show no button (most of the time), or show the haml box (for project validators).
Follow-up work
There is work to do to achieve parity with the old extra links box. This is tracked in CT-550
PR Checklist: