Added template files sync when creating a project from a template #3031
Conversation
abujeda
force-pushed
the
2954_project_template_copy
branch
2 times, most recently
from
61c888e
to
de0ab4e
Compare
| <%- if @templates.size.positive? -%> | ||
| <div class="mt-5 justify-content-center text-center project-icon"> | ||
| <%= link_to(new_project_path({template: true}), | ||
| title: I18n.t('dashboard.jobs_create_template_project'), | ||
| class: 'text-dark btn btn-link') do | ||
| %> | ||
| <%= icon_tag(URI.parse("fas://plus")) %><br> | ||
| <%= I18n.t('dashboard.jobs_create_template_project') %> | ||
| <% end %> | ||
| </div> | ||
| <%- end -%> |
There was a problem hiding this comment.
I consciously made a separate button to distinguish the 2 workflows. I thought it was a better UX when the user knew they were building from a template as apposed to clicking the new button project and wondering what templates where.
I have to admit I'm not a UX expert - I just thought the explicit difference would help beginners.
There was a problem hiding this comment.
Well it really was meant as a question - do you have some UX reason to change it?
I'm just sharpening my UX skills, so again, I'm not really sure - I just thought 2 different flows would limit confusion.
There was a problem hiding this comment.
I would agree if the flows were actually different, and by selecting the template button the user would be sent to a different set of pages. This might be the future state with a more complex template flow, but at the moment, it just adds the template dropdown that is why I thought it was simpler if merged.
There was a problem hiding this comment.
I see. Yea I don't anticipate it being more complex - but again it was a UX driven decision.
I'd say let's keep it the way it is assuming the way it is is a better user experience (which is my assumption, but I'd love if someone could confirm it).
There was a problem hiding this comment.
Updated the PR to keep the template flow as it was.
abujeda
force-pushed
the
2954_project_template_copy
branch
2 times, most recently
from
9185741
to
97e2146
Compare
abujeda
force-pushed
the
2954_project_template_copy
branch
from
97e2146
to
0492402
Compare
| @@ -72,6 +74,7 @@ def templates | |||
| validates :icon, format: { with: %r{\Afa[bsrl]://[\w-]+\z}, allow_blank: true, message: :format }, on: [:create, :update] | |||
| validate :project_directory_invalid, on: [:create, :update] | |||
| validate :project_directory_exist, on: [:create] | |||
| validate :project_template_invalid, on: [:create] | |||
There was a problem hiding this comment.
This isn't necessarily specific to this PR - but why are we using :create here? There's no script#create API.
How does this work when nobody is calling or defined a create API?
There was a problem hiding this comment.
I think I see, we call valid?(:create) somewhere. We can defer this, but that seems confusing.
There was a problem hiding this comment.
I can update it to :save to be consistent with the model method call when validating
There was a problem hiding this comment.
Let's do it in another PR - I think I'm good with this.
Fixes: #2954
First implementation to sync the files from a template into a new project.
All files from the template will be copied in this implementation.
It is based on the batch connect session stage method.
Regarding the template interface, I have removed the link to add a project from a template and always added the templates if any available. Let me know if I should revert this part.