Skip to content

feat: add template activity_bump property #11734

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 13, 2024
Merged

feat: add template activity_bump property #11734

merged 10 commits into from
Feb 13, 2024

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Jan 22, 2024 •
edited
Loading

Adds a new property to templates activity_bump which defaults to 1h to persist the current behavior. Allows the template admin to specify how much time activity bump events should add to the workspace deadline.

TODO:

  • Dashboard: template create
  • Dashboard: template edit

Supercedes #10866
Closes #11690

@deansheather deansheather requested a review from Emyrk January 22, 2024 10:23
Emyrk
Emyrk reviewed Jan 22, 2024
View reviewed changes
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

This only covers the template side of things. Do we not also have to make it editable from workspace?

Just asking, personally I think less toggles is better, and might not be necessary on the workspace.

coderd/database/queries/activitybump.sql Outdated
Comment on lines 71 to 72
-- We only bump if the template has an activity bump duration set.
AND l.activity_bump IS NOT NULL
Copy link
Member

Choose a reason for hiding this comment

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

This can never be null. Should this be = 0 or something?

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 think this didn't trip up tests since this is essentially doing nothing in the query... if it's 0 we're adding 0 to the current deadline anyways lol.

I fixed it anyways since it's better to be safe IMO

@github-actions github-actions bot added the stale This issue is like stale bread. label Feb 1, 2024
@github-actions github-actions bot closed this Feb 5, 2024
@deansheather deansheather reopened this Feb 8, 2024
@deansheather deansheather requested a review from Emyrk February 8, 2024 14:36
@deansheather deansheather removed the stale This issue is like stale bread. label Feb 8, 2024
@deansheather
Copy link
Member Author

This only covers the template side of things. Do we not also have to make it editable from workspace?

Just asking, personally I think less toggles is better, and might not be necessary on the workspace.

@Emyrk I agree, I think we're going to keep this as a template setting for now.

@deansheather deansheather requested a review from johnstcn February 9, 2024 16:29
johnstcn
johnstcn approved these changes Feb 9, 2024
View reviewed changes
Copy link
Member

@johnstcn johnstcn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Backend looks OK. Tested manually by updating deadline to >95% with 5m bump interval.

Some UI issues remain e.g. activity bump shows up as fractional hours
image

@deansheather
Copy link
Member Author

Some UI issues remain e.g. activity bump shows up as fractional hours

Fixed! Apparently most of our validations didn't have .integer() and .required()

@deansheather deansheather enabled auto-merge (squash) February 13, 2024 06:52
@deansheather deansheather merged commit e1e352d into main Feb 13, 2024
@deansheather deansheather deleted the dean/template-activity-bump-dur branch February 13, 2024 07:00
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@johnstcn johnstcn johnstcn approved these changes

@Emyrk Emyrk Emyrk approved these changes

Assignees

@deansheather deansheather

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add inactivity timeout separate from initial TTL
3 participants
@deansheather @johnstcn @Emyrk