-
Notifications
You must be signed in to change notification settings - Fork 886
feat(site): Duplicate template #6853
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
Conversation
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.
Awesome! Noticed a few things:
-
Duplicating a template with no display name will make a display name
Copy
-
Duplicating the built-in docker template returns the following error:
Screen.Recording.2023-03-28.at.2.43.35.PM.mov
I tried importing another template with coder templates init
(using Docker template) and ran into the same one. I tried with both localhost:3000
and localhost:8080
. Any idea why this is happening?
Is it ok if we just left the field blank?
I will take a look into that 🤔 |
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.
just a side note: do we need to check user template permissions here? so that only template authors can copy the template.
@mtojek Yes! Good catch. |
@bpmct do you mind taking a look into it and see if that works as expected now pls? |
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.
The code logic is 👍 . My biggest concern is the lack of consistency between the public name and codebase: Duplicate a template
vs. copy
. I think that you should unify this, and just stick to duplicate
. I know that these are synonyms, but it will be easier to work with code later.
I fixed the issues that you brought! Waiting for a new review.
Screen.Recording.2023-03-28.at.13.41.12.mov
I'm going to work on the tests when the flow is validated from @bpmct
Closes #6118