DOP-4274: Label inactive branches in Slack Deploy Modal #965
Conversation
|
Your feature branch infrastructure has been deployed! Your webhook URL is: https://5aj73wwjea.execute-api.us-east-2.amazonaws.com/prod/webhook/githubEndpoint/trigger/build For more information on how to use this endpoint, follow these instructions. |
anabellabuckvar
changed the title
api/handlers/slack.ts
Outdated
| @@ -24,14 +24,19 @@ export async function buildEntitledBranchList(entitlement: any, repoBranchesRepo | |||
| const [repoOwner, repoName, directoryPath] = repo.split('/'); | |||
| const branches = await repoBranchesRepository.getRepoBranches(repoName, directoryPath); | |||
| for (const branch of branches) { | |||
| let buildWithSnooty = true; | |||
| if ('buildsWithSnooty' in branch) { | |||
There was a problem hiding this comment.
I know this is a holdover from the past, but I think we can get rid of this conditional. It doesn't gate anything and we're just checking true/false which undefined will work for as well.
api/handlers/slack.ts
Outdated
| branch['gitBranchName'] | ||
| }`; | ||
| if (!active) { | ||
| entitledBranches.push(`!Inactive ` + `${repoPath}`); |
There was a problem hiding this comment.
total nit: Because of the font-style of Slack, I think it might be more readable with a lower-case i
There was a problem hiding this comment.
And up for debate - I'm wondering how to make it the most pleasing when we have no access to style. I wonder if it would look better in parentheses?
(!inactive) 10gen/docs/master
wdyt?
There was a problem hiding this comment.
Oh another nit: since you're already using template strings, you can get rid of the concatenation!
(!inactive) ${repoPath}
There was a problem hiding this comment.
I definitely think in parentheses would look better! I was following the ticket exactly but thats a great idea, thank you; I'll confirm in the channel
** UPDATE: Cassidy confirmed parentheses
src/services/slack.ts
Outdated
| const fullBranchPath = fullPath; | ||
| const displayBranchPath = fullPath; | ||
| let valueBranchPath = fullPath; | ||
| const inactiveStr = /!inactive/gi; | ||
| const isInactive = fullPath.search(inactiveStr); | ||
| if (isInactive != -1) { | ||
| valueBranchPath = fullPath.slice(10); | ||
| } | ||
| const opt = { | ||
| text: { | ||
| type: 'plain_text', | ||
| text: fullBranchPath, | ||
| text: displayBranchPath, | ||
| }, | ||
| value: fullBranchPath, | ||
| value: valueBranchPath, |
There was a problem hiding this comment.
Maybe we can use a quick startsWith method to make this more legible. Link to MDN
anabellabuckvar
changed the title
.github/workflows/deploy-stg-ecs.yml
Outdated
| @@ -3,6 +3,7 @@ on: | |||
| branches: | |||
| - "master" | |||
| - "integration" | |||
| - "DOP-4274" | |||
There was a problem hiding this comment.
Just a reminder to remove this before merging
Context
It is very easy to accidentally deploy inactive versions in the Slack dropdown at /deploy - they need to be deployable to enable corrective edits or deploy-related platform functions, but there isn't visibility on which are inactive within the modal.
DOP-4274
Staging
The changes were tested and staged in preprd. Inactive branch entries are labeled within the /deploy and /test_deploy UI.