PLU-743: [IF-THEN-THEN-V2-17] Deprecate branch name - #2002
Conversation
38dffaa to
4a91d4d
Compare
Code reviewFound 2 issues:
plumber/packages/frontend/src/helpers/getStepName.ts Lines 38 to 42 in 4a91d4d
The third line ("we combine them if thats the case") just narrates the plumber/packages/frontend/src/helpers/getStepName.ts Lines 26 to 30 in 4a91d4d 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
There was a problem hiding this comment.
lgtm, tested works! few things to clarify:
- branch/step name:
- should we update the
stepNametobranchNamefor all branches when upgrading from v1 to v2? feels like better UX than to only have some updated and some not - small thing: focusing on the step name, then hitting enter without typing anything causes us to end up with
Branch 1 (Branch 1)(assuming that 'Branch 1' is the original branch name) - feels a little odd to type a new step name -> save -> see the name as
branchName (newStepName)(might not be worth it to fix if complex logic is needed)
- should we update the
- small ordering thing with MRF
- if there are no actions between MRF step 1 and MRF step 2 -> cannot draft the if-then in between
- add an action between MRF step 1 and MRF step 2 -> can drag if-then inside
- removing MRF steps
- deleting MRF steps deletes other actions that are below the MRF workflow steps, is this intended?
- we might also want to update the templates to immediately adopt v2?
4a91d4d to
e5d7edf
Compare
e5d7edf to
efc791b
Compare
…ame for leftover V1 Hides Branch Name once the owner's V2 flag is on, makes it optional backend-side, folds it into leftover V1 steps' name, and fixes an emptied custom name rendering blank (default renamed to "If-then"). # Conflicts: # packages/frontend/src/components/FlowSubstep/index.tsx
efc791b to
554294e
Compare

Context
We are enabling steps to be inserted after If-thens. The main changes:
endStepIdmarker in each if-then step, so that we know when the conditional steps end.endStepIdis set to the if-then's step ID.Important
Important
This new If-Then is called If-Then V2 in the codebase; we don't want to make a big bang change.
This PR
Deprecates the
branchNameparameter for If-Then V2. High level approach:branchNamean optional param (we expect to move people to V2 ASAP so this shouldn't be too big of a problem)argsToDisplayto hide branch name - this is bad (tm) but I'll clean it up once v2 is rolled out fully.Edge case: a branch might have both step name and branch name. In this case, we display it as
${branchName} (${customStepName}).Tests
This is the top stack in the PR, so here are the tests:
With LD flag OFF
stepIdToJumpTois not populatedstepIdToJumpTois not populated.With LD flag ON
stepIdToJumpTois populated.stepIdToJumpTois populated.stepIdToJumpTo), and that the added steps run outside of the If-Then conditionalstepIdToJumpTo), and that the added steps run outside of the If-Then conditional