I don't think that would fix it. The attribute can be used to limit the base of audits that we are searching for - e.g. only templates which content has changed, but the method still checks whether name has changed at the end.
So it seems we have +1 from everyone so far, I'll try to find some time and send a PR this week. Thanks everyone involved -- Marek On úterý 21. února 2017 8:56:15 CET Stephen Benjamin wrote: > Oh :-( > > It looks like the fix would be just to pass the `template` attribute > to audit_modified?, it can check other attributes than name. > > But locking the templates we ship would be preferable to me. > > > - Stephen > > On Fri, Feb 17, 2017 at 9:04 AM, Marek Hulán <mhu...@redhat.com> wrote: > > Hello foreman-devs, > > > > recently I was told about the bug that we override all templates in > > database whenever we run db:seed. From the code [1] and commit message > > [2], it was not the intended behavior. It was supposed to check whether > > user made some changes and only apply the new version if the template was > > not touched. Sadly, the method only checks the name attribute for changes > > [3], so if "only" template content was changed, we still override it. > > > > While I can try to fix it to originally intended behavior, I'd like to ask > > whether it wouldn't be better to use this opportunity and start locking > > templates we ship by default. The recommended workflow for users would be > > to clone the template if custom changes are needed. We'd always update > > locked templates. Obviously, user would need to merge new version to > > cloned template on his own. With foreman_templates plugin it should be > > easy enough to export templates and see the diff between default and > > customized template, apply the changes user wants and then reimport them > > back. > > > > I think this would be overall better user experience and safer workflow. > > The originally intended behavior would never update a template that user > > modified. That means after update user ends up with template from old > > Foreman version (with custom changes) that might not be compatible with > > the new Foreman version. This is more likely to happen than before > > because we now version templates in community-repo and we don't keep > > backward compatibility as we did before. > > > > Thanks for reading, thoughts? > > > > [1] > > https://github.com/theforeman/foreman/blob/1.14.0/db/seeds.d/07-provision > > ing_templates.rb#L98 [2] https://github.com/theforeman/foreman/commit/ > > d4ed70154fa9f6c83597adc784240e3865845563 > > [3] https://github.com/theforeman/foreman/blob/1.14-stable/db/seeds.rb#L33 > > > > -- > > Marek > > > > -- > > You received this message because you are subscribed to the Google Groups > > "foreman-dev" group. To unsubscribe from this group and stop receiving > > emails from it, send an email to > > foreman-dev+unsubscr...@googlegroups.com. For more options, visit > > https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
