- Notifications
You must be signed in to change notification settings - Fork921
fix: remove cancel button if user cannot cancel job#11553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -55,7 +55,7 @@ export interface WorkspaceProps { | |||
| workspaceErrors: WorkspaceErrors; | |||
| buildInfo?: TypesGen.BuildInfoResponse; | |||
| sshPrefix?: string; | |||
| template?: TypesGen.Template; | |||
| template: TypesGen.Template; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why did we change this type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I noticed it didn't need to be conditional and see those as a liability so I just drive-by fixed it while I was reading the code.
Kira-Pilot left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for making this change! Looks good to me.
Can we add a storybook test before we merge it up? Your screenshots would be good test cases.You'll need to mock the user value theuseMe hook returns and the way to do this is to mock out theAuthContext.Provider
Here's an example of how we mock providers in a Storybook file.
As discussed over Slack, we'll need to move this hook invocation up to fix the broken stories. Since you'll be accessingisOwner as a prop, you won't need to mock that auth provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good!
Uh oh!
There was an error while loading.Please reload this page.
Closes#11449
The user can only cancel under the following circumstances according to the backend:
OR
Owner View if user cancelation is disabled:
Member View if user cancelation is disabled:
