Skip to content
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

New template creates duplicate #9817

Closed
CodeartIT opened this issue Feb 15, 2021 · 15 comments
Closed

New template creates duplicate #9817

CodeartIT opened this issue Feb 15, 2021 · 15 comments
Labels
community/up-for-grabs status/stale Marked as stale due to inactivity

Comments

@CodeartIT
Copy link

CodeartIT commented Feb 15, 2021

When I want to create new template for masterpage and If I am using space in template name, for example "Master page", Umbraco will create 2 new templates with same name, sometimes umbraco creates 2 templates with same nameeven if I dont use space in name, but I am not sure why is that, because sometimes it works normal and sometimes not.

Update: My steps to reproduce sometimes dont work, so maybe space is not the reason. I am not shure what is the problem, but sometimes Umbraco deide to create duplicate template, I am not shure what is the reason.

Umbraco version

I am seeing this issue on Umbraco version: 8.11.1

Reproduction

Steps to reproduce

  1. Create new template
  2. Give it a name with space for example "Master page"
  3. Save

Expected result

Umbraco should create 1 new template with given name

Actual result

Umbraco creates 2 templates with same name

@nul800sebastiaan
Copy link
Member

Hi @CodeartIT, I have not seen multiple templates being created with the same name, do you mean in the Templates tree in the backoffice? And does Umbraco add (1) to the second template?

I just save that we have this weird behavior where a template is saved as soon as the name field loses focus, I can't remember why this is implemented in this way, but that could definitely have something to do with this problem.

9817

In any case, I think this behavior above should be changed and we should only save if you actually trigger the "Save" button. We'd love some help with that! I suspect that would also fix this problem you're experiencing.

@umbrabot
Copy link

Hi @CodeartIT,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

@bjarnef
Copy link
Contributor

bjarnef commented Mar 5, 2021

I can confirm this issue in Umbraco v8.11.1
E.g. enter a name and hit save button. It seem it creates a template both on blur (which probably trigger a save) and clicking Save button.

The first one has alias Master the second has alias Master1.

image

@idseefeld
Copy link
Contributor

I confirm for version 8.12.2.

@bjarnef
Copy link
Contributor

bjarnef commented Apr 1, 2021

I would say it should never create templates om blur, but require an active save like everything else in backoffice UI. Not sure the reason behind thr save on blur was implemented as it previous only saved in button click or via shortcut key.

@bjarnef
Copy link
Contributor

bjarnef commented Apr 2, 2021

It was implemented as part of f9761ad

@nathanwoulfe do you recall why we need to bind the on blur event on name input field and save the template on blur?
It seems to be related to avoid overwrite existing template on disk, but e.g. when clicking save button it trigger a double-save: A save on blur and a second save on the button click.

@bjarnef
Copy link
Contributor

bjarnef commented Apr 2, 2021

Maybe we should instead implement an optional onNameBlur instead similar to onBack?

@nathanwoulfe
Copy link
Contributor

The save on blur was added to avoid overwriting when a template file exists on disk but not in the backoffice.

Without the blur event, the existing file would be overwritten with whatever the editor adds in the backoffice.

Rather than a save on blur, should really just check for the existing file, and offer to load it. There may have been a requirement for saving, was a while ago so my memory isn't crystal clear.

@nathanwoulfe
Copy link
Contributor

Alternatively we could debounce the save, I think the blur would still fire before the save click, so could suppress the button click if the blur has fired and not yet had a response

@bjarnef
Copy link
Contributor

bjarnef commented Apr 3, 2021

@nathanwoulfe I still wonder if we should handle this another way than save in blur, since we don't do that elsewhere in backoffice UI.

Maybe the save should instead check for existing template instead before creating a new.

@nathanwoulfe
Copy link
Contributor

I don't remember if there was a reason we did the save this way, but it doesn't really make sense, especially since it's not done elsewhere...

@bjarnef
Copy link
Contributor

bjarnef commented Apr 3, 2021

I think it comes down to SaveFile which create the view on the disk.
https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/Persistence/Repositories/Implement/TemplateRepository.cs#L237-L258

Whether view exists or not on disk and when creating a new template from backoffice, the originalAlias is null and it will then use _viewHelper.CreateView(template, true).

It could check here if the file exists on disk or maybe it should be handled in CreateView method?

image

@bjarnef
Copy link
Contributor

bjarnef commented Apr 3, 2021

Actually it seems CreateView does check if the file exists:
https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Core/IO/ViewHelper.cs#L46

But the second parameter overwrite any existing file on disk, so it seems it works as expected.
I wonder if it should just use _viewHelper.CreateView(template, false) or _viewHelper.CreateView(template) instead?

@bjarnef
Copy link
Contributor

bjarnef commented Apr 3, 2021

I have submitted a PR #10098
It seems to be more reliable how the template is saved 👍

@umbrabot
Copy link

Hiya @CodeartIT,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant
This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@umbrabot umbrabot added the status/stale Marked as stale due to inactivity label Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community/up-for-grabs status/stale Marked as stale due to inactivity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants