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

Diff becomes outdated if original template is changed/updated #10

Open
julianlam opened this issue Jul 5, 2022 · 4 comments
Open

Diff becomes outdated if original template is changed/updated #10

julianlam opened this issue Jul 5, 2022 · 4 comments

Comments

@julianlam
Copy link
Member

This may be functioning as intended, but I wanted to raise it nonetheless.

When a customized template is added, two values are saved into the database, old and diff, referring to the original template, and the changes made.

If the underlying template gets changed, old and diff no longer reflect the proper changeset, and could cause issues if the original template contains breaking changes. This is because diff is applied onto old, and then saved to the templates folder, so it will never inherit the updated template.

e.g.

  • Persona templates/partials/chats-menu.tpl contained changes, it now contains tabs and shows profile options, notifications, as well as chats.
  • If that partial was changed in any way, chats-menu.tpl will always contain the old modified template, and will not inherit any new changes to that partial.
@julianlam
Copy link
Member Author

The obvious fix is to update old and diff if the underlying template has changed.

However, I understand the rationale behind not doing so, in that updating the diff would be confusing since it would then contain changes that were not necessarily part of the original changeset.

A compromise may be a UI warning to instruct the user that the underlying template has changed, and so the customized template may need to be re-customized.

@pitaj
Copy link
Collaborator

pitaj commented Jul 5, 2022

I think ideally, we'd save the original when the diff was created, and we'd allow the user to compare that to the current template, and apply the old diff to the new template.

There are a few hurdles to overcome. Right now we pull templates that have already been built. On a rebuild, now our modified template is there in it's place.

So it's hard to even find out if our "original" is modified. To fix this, we need to have some way to ask NodeBB "what plugin / theme is this template from" and fetch the original and current versions from there instead.

This would also provide the benefit of not baking in partials, which would probably make things a bit easier for users.

@julianlam
Copy link
Member Author

It looks like right now you just read through all the tpl files and allow them to be customized.

A core helper method to return the path (and optionally the raw template itself without partials baked in) sounds like it would help you refactor this.

@julianlam
Copy link
Member Author

... and an md5sum, why not...!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants