Skip to content

fix(site): disable autostart and autostop according to template settings #11809

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 26, 2024

Conversation

Kira-Pilot
Copy link
Member

@Kira-Pilot Kira-Pilot commented Jan 24, 2024

resolves #10930
resolves #10931
Deployed a PR below

Disabling these fields on a template level:
Screenshot 2024-01-24 at 2 02 33 PM

will disable them on a workspace level (but will not reset any workspace values set before disablement):
Screenshot 2024-01-24 at 2 02 44 PM

Similarly, when specific days of the week are allowed to be configured per the template:
Screenshot 2024-01-24 at 2 04 48 PM

only those days of the week are enabled in the workspace settings:
Screenshot 2024-01-24 at 2 05 02 PM

@Kira-Pilot Kira-Pilot changed the title fix (site): disable autostart and autostop according to template settings fix(site): disable autostart and autostop according to template settings Jan 24, 2024
@@ -311,14 +315,14 @@ export const WorkspaceScheduleForm: FC<
<Stack direction="row">
<TextField
{...formHelpers("startTime")}
disabled={isLoading || !form.values.autostartEnabled}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why we were looking at the form values here...I assume this was a typo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because that's where the state of the form is. So if someone turns on autostart, this is the place to check its value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the disabled state matches their checkbox

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching me here - I've fixed the attribute and added some test cases.
Down the road, we might want to reconsider the prop architecture of the form as it accepts initialValues, and one could try to disable autostart or autostop using this prop, e.g. :

initialValues={{
  autoStartEnabled: false
  autoStopEnabled: false
}}

But this wouldn't affect the top toggle of each feature; only the secondary fields associated with them. So it's a bit of a bizarre API, but I'd rather not touch it in this PR.

Copy link

github-actions bot commented Jan 24, 2024


✔️ PR 11809 Updated successfully.
🚀 Access the credentials here.

cc: @Kira-Pilot

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1 for tests!

@ericpaulsen
Copy link
Member

thank you for this @Kira-Pilot - wanted to check whether this PR also addresses the below:

in addition, the changes to the autostart selections are not persisted when navigating to another template tab, such as Parameters, even after clicking Submit.

@matifali
Copy link
Member

matifali commented Jan 26, 2024

That's great @Kira-Pilot . I noticed the week start day is different on Template and Workspace settings. It starts with Sunday vs Monday. Could you also align the week start day here?

@Kira-Pilot
Copy link
Member Author

@matifali fixed!
@ericpaulsen thanks for pointing this out; I overlooked this part of the ticket. Fixed!

@Kira-Pilot Kira-Pilot merged commit 4c71ccc into main Jan 26, 2024
@Kira-Pilot Kira-Pilot deleted the disable-auto-start_stop/kira-pilot branch January 26, 2024 14:06
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants