Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(site): exclude workspace schedule settings for prebuilt workspaces#18826

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

Merged

Conversation

ssncferreira
Copy link
Contributor

@ssncferreirassncferreira commentedJul 10, 2025•
edited
Loading

Description

This PR updates the UI to avoid rendering workspace schedule settings (autostop, autostart, etc.) for prebuilt workspaces. Instead, it displays an informational message with a link to the relevant documentation.

Changes

  • IntroduceIsPrebuild parameter toconvertWorkspace to indicate whether the workspace is a prebuild.
  • Prevent the Workspace Schedule settings form from rendering in the UI for prebuilt workspaces.
  • Display an info alert with a link to documentation when viewing a prebuilt workspace.
Screenshot 2025-07-10 at 13 16 13

Relates with:#18762

BrunoQuaresma reacted with thumbs up emoji
@ssncferreirassncferreiraforce-pushed thessncferreira/exclude-schedule-settings-for-prebuilds branch fromc6e4ff3 to95ca5f7CompareJuly 10, 2025 12:19
@ssncferreirassncferreira marked this pull request as ready for reviewJuly 10, 2025 12:32
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest adding a story here to showcase the new behaviour.

BrunoQuaresma reacted with thumbs up emoji
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch!

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Addressed in73f5476

Copy link
Collaborator

@BrunoQuaresmaBrunoQuaresma left a comment

Choose a reason for hiding this comment

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

FE changes look good to me 👍. Just left a comment related to naming. Also, would be nice to add a story for this use case as Cian mentioned#18826 (comment).

Copy link
Contributor

@SasSwartSasSwart left a comment

Choose a reason for hiding this comment

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

Looks great! I've added a few nonblocking thoughts for your consideration.

@@ -2231,6 +2231,12 @@ func convertWorkspace(
if latestAppStatus.ID == uuid.Nil {
appStatus = nil
}

isPrebuild := false
Copy link
Contributor

@SasSwartSasSwartJul 14, 2025•
edited
Loading

Choose a reason for hiding this comment

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

A few nonblocking thoughts:

  • ConsiderisPrebuilt instead ofisPrebuilt. I know is a small and subtle naming change but a Prebuild is not the same thing as a prebuilt workspace.
  • Also perhaps consider a comment or something to explain that this is for unclaimed prebuilt workspaces only. A claimed workspace can still be called prebuilt and the change in this PR does not affect those.
  • should we have aprebuilds.IsUnclaimedPrebuiltWorkspace(...) function to encapsulate and abstract this logic?

Copy link
ContributorAuthor

@ssncferreirassncferreiraJul 14, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Consider isPrebuilt instead of isPrebuilt. I know is a small and subtle naming change but a Prebuild is not the same thing as a prebuilt workspace.

The way I have been using these terms, isPrebuild =Prebuilt Workspace, but maybe I have been using it wrong. What is the difference between Prebuild and Prebuilt Workspace?

Also perhaps consider a comment or something to explain that this is for unclaimed prebuilt workspaces only. A claimed workspace can still be called prebuilt and the change in this PR does not affect those.

Added a comment in0bde254, let me know if it works.

should we have a prebuilds.IsUnclaimedPrebuiltWorkspace(...) function to encapsulate and abstract this logic?

Yes, we can do that and reuse it in other parts of the code that already have this logic. I can address it in a separate PR. Edit: actually, there is a better solution that I completely forgot 🤦‍♀️ which is to useworkspace.IsPrebuild(). This encapsulates that logic onworkspace and improves readability:0120c5b
I will create a follow up PR to add some tests to test this parameter.

@ssncferreirassncferreira merged commitdad033e intomainJul 15, 2025
35 of 37 checks passed
@ssncferreirassncferreira deleted the ssncferreira/exclude-schedule-settings-for-prebuilds branchJuly 15, 2025 13:11
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJul 15, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn left review comments

@BrunoQuaresmaBrunoQuaresmaBrunoQuaresma approved these changes

@SasSwartSasSwartSasSwart approved these changes

Assignees

@ssncferreirassncferreira

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@ssncferreira@BrunoQuaresma@johnstcn@SasSwart
[8]ページ先頭
©2009-2025 Movatter.jp