FEM-compatible Project Builder (aka Lab 2 aka /lab-fem)

[shaunanoordin]

PR Overview

This PR adds an FEM-compatible Project Builder to PFE.

Context: as we try to migrate fully from PFE to FEM, we need to encourage project owners to build projects that are compatible with FEM. To do so, we need to give them a Project Builder that's compatible with FEM.

• The FEM-compatible Project Builder is accessible from the /fem-lab route.
• Code is based on the original Project Builder in the adjacent ./lab folder.

Testing

Staging URL: https://pr-6037.pfe-preview.zooniverse.org/

Suggested Test URLs:

• https://pr-6037.pfe-preview.zooniverse.org/lab-fem/
• https://pr-6037.pfe-preview.zooniverse.org/lab-fem/1873/workflows/3520

Status

WIP

Completed:

• Added stripped-down Workflow Editor from the earlier lab branch that Jim worked on, plus its styles
• Added new /lab-fem route (we can change this to /lab-2 later if we decide to)
• Example: https://local.zooniverse.org:3735/lab-fem/1873/workflows/3520
• UPDATE: we no longer explicitly split the FEM Lab and the PFE Lab via an explicit URL. Instead, the same PFE-compatible OR FEM-compatible workflow editor page is accessible from https://local.zooniverse.org:3735/lab/1873/workflows/3520 . Whether or not you get the PFE-compatible or FEM-compatible version depends on the underlying project, and whether it's configured to use FEM.

TODO:

• Shortcuts Editor still needs to be added to match the lab branch.
• Holy crap a lot of the workflows component has the /lab path baked into its links, this needs to be solved.

[coveralls]

Coverage increased (+0.07%) to 50.56% when pulling eec8cf1 on lab-2 into 4aa90b9 on master.

[shaunanoordin]

PR Update

So it turns out that a lot of the code in the app/pages/lab folder has the /lab route/path pretty much hardcoded into the code. (e.g. workflow-container.jsx's labPath() function returns "/lab/${this.props.project.id}${postFix}") This makes surgically copying only a limited subset of app/pages/lab/* files kinda tricky.

As a result, here's my new approach: copy the WHOLE app/pages/lab folder into app/pages/lab-fem, THEN make the necessary changes for FEM-compatibility, AND write the syncing instructions (including what necessary changes to make) into the README.

Dev Notes

@mcbouslog apologies for going down this route; I wanted to make minimal copy-pastes to make the review process MUCH easier, but this was the best way I could find to compartmentalise the /lab-fem changes.

• The best way to actually compare the code isn't to use the github diff to compare branches, but to compare directories via diff app/pages/lab app/pages/lab-fem
• Notable changes in app/router.cjsx
• Notable changes in app/pages/lab-fem/workflow.cjsx, but that needs to be compared with app/pages/lab/workflow.cjsx

(Note: the alternative to copy-pasting the whole folder was to modify the existing lab code to have an if-else switch, so project.cjsx's @context.router.push '/lab' becomes @context.router.push getLabOrLabFemUrl('/'), with a new getLabOrLabFemUrl() function that checks window.location.pathname)

Status

Small WIP left - I need to modify the shortcut editor

[eatyourgreens]

I'm probably missing something but why not pass the base path (/lab or /lab-fem) into the project builder as a prop?

[eatyourgreens]

You can also get the current path from React router, which should tell you the path prop for the current component. That might be more useful here, because router context is available to any component.

[shaunanoordin]

PR Update

❗ Big change: whether or not your get an FEM-compatible or PFE-compatible Project Builder page is now determined by the underlying project's configuration, NOT an explicit URL.

e.g. https://pr-6037.pfe-preview.zooniverse.org/lab/1873/workflows/3532 will give you the FEM-compatible workflow editor, IF project 1873 is configured to use FEM. Otherwise, you'll get the PFE-compatible workflow editor.

• FEMLabRouter component added to perform the splitting logic. This 'splitter' returns a different component, depending on the path AND whether or not the associated Zooniverse Project is FEM-compatible.
• A LOT of massive code changes have been simplified, woo hoo!
• At the moment, the "should this project use FEM-compatible pages?" configuration flag is... supremely arbitrary. (As of writing, the check is just "is this Project 1873?" ) I'll address this next. See Slack convo for ongoing discussion on where the config flag should be placed.

[shaunanoordin]

PR Update

• The "should this project use FEM-compatible pages?" check is now determined by the following:
  • Is the project's slug registered in the PFE code?
  • Is the ?femLab=true flag set? e.g. https://local.zooniverse.org:3735/lab/1873/workflows/3532?femLab=true
• There is an alternate override, ?pfeLab=true, to force using the PFE-compatible lab pages.

I'm going to mark this PR ready for review, but I want to have one additional quick chat with Cliff on Slack to see if he wants to have an easier toggle for enabling/disabling FEM lab mode (i.e. use Experimental Tools while we wait for the full top-level projectResource.useFEM flag to be set up.

[shaunanoordin]

PR Update (Ready-For-Review Compilation)

This PR adds an FEM-compatible Project Builder to PFE.

• New behaviour: the Project Builder will automatically switch between FEM-compatible or PFE-compatible pages, depending on the project being edited.
  • e.g. https://pr-6037.pfe-preview.zooniverse.org/lab/1873/workflows/3532 will give you the FEM-compatible workflow editor, IF project 1873 is configured to use FEM. Otherwise, you'll get the PFE-compatible workflow editor.

At the moment, this is how the FEMLabRouter determine if we should use FEM-compatible pages:

• Use FEM Lab if ANY one or more of the following conditions are met:
  • Project is already using the FEM Classifier (i.e. has its slug registered in the monorepo routes)
  • Project has experimental_tools.femLab = true
  • ?femLab=true query param is set
• Use PFE Lab in all other cases, OR if ?pfeLab=true query param is set.

UPDATE: you can now set the femLab experimental tool flag via the Admin page.

Context: as we try to migrate fully from PFE to FEM, we need to encourage project owners to build projects that are compatible with FEM. To do so, we need to give them a Project Builder that's compatible with FEM.

Dev Notes

• Switching logic is found in FEMLabRouter.jsx
• app/pages/fem-lab/workflow.cjsx is just a modified version of app/pages/lab/workflow.cjsx

Status

This PR is now ready for review

[shaunanoordin]

@lcjohnso @mrniaboc @snblickhan

Executive Summary

• With the current incarnation of the PR, the PFE Project Builder will automagically switch between PFE-compatible pages and FEM-compatible pages.
• The project builder at zooniverse.org/lab/1234/workflows/5678 will use FEM-compatible Lab pages ONLY if the following condition is met:
  • Project has experimental_tools.femLab = true (this can be set via Admin pages)
  • Project is already using the FEM Classifier (e.g. Planet Hunters TESS) EDIT: removed

We can also add ?pfeLab=true or ?femLab=true to the URL to override the automagic switching. Please only use to debug problems during this early migration efforts; I plan to remove these flags eventually.

Long term, I DO believe there's agreement among the devs to use a project-level project.plsUseFEM = true attribute. (I think that's stalled for the moment because it's all tied up on how to also route the FEM Classifier?) The experimental tool solution in this PR is just something to get this PR out the door. 👌

Here are the pros/cons of our current approach:

• ➕ We can easily let project owners start building/editing FEM-compatible projects by toggling the FEM Lab.
• ✖️ Unfortunately, the project won't have access to the FEM Classifier, since that still requires devs to set up routes on PFE and FrontDoor, etc.
• ✖️ Also, without the FEM Classifier, the project owners can't test their projects. EDIT: wait I just realised I can route project owners to https://frontend.preview.zooniverse.org/ for testing. Huh! Should have thought of that earlier. OK, that's my next PR.
• ➕ When we finally have the project-level attribute set up, we can very easily "transfer" project.experimental_tools.femLab = true to project.plsUseFEM = true
• ❗ Did I mention that all existing FEM projects will now use the FEM compatible Lab ? Do we need to give them a heads up? EDIT: removed

[shaunanoordin]

Blargh, the staging deploys are still borked since this morning, so it's difficult to test the code on https://pr-6037.pfe-preview.zooniverse.org/ until it's fixed.

[shaunanoordin]

Updated branch to reflect changes to workflow.cjsx in #6042

[lcjohnso]

@shaunanoordin -- Following up on our conversation today -- a few opinions:

1. Do not auto-opt-in projects to using lab-fem that are already redirecting to FEM. We want to finalize the FEM project builder interface changes before enabling for any projects.
2. Both the "View Project" and "Test This Workflow" buttons should point to FEM. Specifically, the fe-project.zooniverse.org domain.

[eatyourgreens]

Can we use front end.preview.zooniverse.org for links, so that JS, images etc are served via the CDN?

[shaunanoordin]

@eatyourgreens I wanted to confirm something that I was discussing with Cliff earlier. Can you please help confirm my understanding that...?

• fe-project.zooniverse.org points to the production-release version of FEM
• frontend.preview.zooniverse.org points to the master version of FEM

If what I talked with Cliff is correct, then the idea to use fe-project.zooniverse.org is to ensure project owners don't accidentally get a preview of "merged but not yet deployed" patches.

[shaunanoordin]

Also, following Cliff's comments:

PR Update

• all existing FEM projects will not automatically use the FEM compatible Lab.
  • This means we need to remember enable, e.g. planetHuntersTessProject.experimental_tools = ['femLab']
  • Code-wise, I've removed the usesMonorepo() clause in FEMLabRouter
• Noted that the "View Project" and "Test this Workflow" should both point to the FEM preview URL.
  • This will be done in a separate PR, as discussed.

I've updated the Executive Summary with some notes.

[eatyourgreens]

Oh good point. fe-project bypasses our proxying so doesn’t quite match the production setup either. Probably not a big deal, but you can’t go to Talk, for example.

[mcbouslog]

This looks great!! Confirmed status quo, and...

• PFE project => PFE project builder
• PFE project => toggle experimental FEM builder => FEM builder
• PFE project => query param to FEM builder => FEM builder
• FEM project => query param to PFE builder => PFE builder

Requesting changes relating to query param check and omitting ShortcutEditor, but open to pushback.

[shaunanoordin]

PR Update

Thanks for the review, @mcbouslog ! This PR has been updated with your feedback:

• ShortcutEditor is now completely removed from FEM-Lab. Since the shortcut feature was hidden behind experimental_tools, it should be safe to remove completely, instead of having the messy workaround code just to disable + plonk a "Deprecated" label on top of the feature)
• ?pfeLab and ?femLab are now more strict about checking for the string "true", instead of checking for the presence of any string

👍

[mcbouslog]

LGTM