Duplicate Outdated template check too strict

[Jon W]

Affected version

2.0.9

The outdated template check compares the date of parent templates with the date of any modified child templates. Currently, if the child template was created at or earlier than the last modified time of the parent template it is considered outdated.

In the situation that a parent template and child template are created automagically at exactly the same time, this means it is automatically marked as outdated, which seems wrong.

It would be better if the check was only for "earlier than" and not "at or earlier than", i.e. the query in the getBaseOutdatedTemplateData() method should change from:

        return $db->fetchAllKeyed('
            SELECT template.template_id,
                parent.version_string AS parent_version_string,
                parent.last_edit_date AS parent_last_edit_date,
                IF(parent.version_id > template.version_id, 1, 0) AS outdated_by_version,
                IF(parent.last_edit_date > 0 AND parent.last_edit_date >= template.last_edit_date, 1, 0) AS outdated_by_date
            FROM xf_template AS template
            INNER JOIN xf_style AS style ON (style.style_id = template.style_id)
            INNER JOIN xf_template_map AS map ON (
                map.style_id = style.parent_id
                AND map.type = template.type
                AND map.title = template.title
            )
            INNER JOIN xf_template AS parent ON (map.template_id = parent.template_id
                AND (
                    (parent.last_edit_date > 0 AND parent.last_edit_date >= template.last_edit_date)
                    OR parent.version_id > template.version_id
                )
            )
            WHERE template.style_id > 0
            ORDER BY template.title
        ', 'template_id');

to:

        return $db->fetchAllKeyed('
            SELECT template.template_id,
                parent.version_string AS parent_version_string,
                parent.last_edit_date AS parent_last_edit_date,
                IF(parent.version_id > template.version_id, 1, 0) AS outdated_by_version,
                IF(parent.last_edit_date > 0 AND parent.last_edit_date > template.last_edit_date, 1, 0) AS outdated_by_date
            FROM xf_template AS template
            INNER JOIN xf_style AS style ON (style.style_id = template.style_id)
            INNER JOIN xf_template_map AS map ON (
                map.style_id = style.parent_id
                AND map.type = template.type
                AND map.title = template.title
            )
            INNER JOIN xf_template AS parent ON (map.template_id = parent.template_id
                AND (
                    (parent.last_edit_date > 0 AND parent.last_edit_date > template.last_edit_date)
                    OR parent.version_id > template.version_id
                )
            )
            WHERE template.style_id > 0
            ORDER BY template.title
        ', 'template_id');

 

[Chris D]

In the situation that a parent template and child template are created automagically at exactly the same time, this means it is automatically marked as outdated, which seems wrong.

I think the key question is, what would actually cause that situation?

Technically it's not possible to do that, so the issue arises essentially if you're doing something unexpected. In theory, then, it's not a bug and it's behaving as designed. But if you can provide more detail about the use case we'll give it some thought.

 

[Jon W]

The situation is exactly that -- an add-on is generating templates automatically for all the styles at exactly the same time. In this case it is a stylesheet.

I could workaround it by giving false dates and forcing it not to say it is outdated, but that seems wrong.

I guess in theory someone very very fast could create a parent template and a child template in the correct order within the space of a second and still get this error, so in that sense it is a bug. I can't think of any case where having the equals to would be desirable.

 

[Jon W]

I guess the really really fast person could also do it the wrong way round. Bad example!!

 

[Lukas W.]

I don't think that in any possible situation marking them as outdated when the date is the same would be the right action. Imo Jon has described the two basic scenarios that lead to this situation: Code generates the templates within a split second difference in time, or a user quickly saves the templates at exactly the same time. Both scenarios seem to be right to me, so there'd be no reason to mark the templates as outdated.

 

[Chris D]

The change is problematic and I'm still not certain of the exact use case, i.e. why it's necessary to create a parent template and child templates together. It still strikes me as a highly unlikely and frankly unusual use case, but I don't have the full information so it's difficult to be certain.

The reason the change is problematic is actually exactly what you mention, and we would argue that outdating the template, in this case, would be correct.

Let's say you have two different users who are both working on the same template in a parent and child style. You're right that if they both made the same changes at the same time then outdating it would be incorrect. But if they both made different changes at the same time then the child would not include the changes from the parent and therefore by definition, the child template is outdated.

After talking to Mike, we think this has been reported before, by @Jake B.

https://xenforo.com/community/threa...-last_edit_date-is-the-same-as-parent.137292/

We still suggest finding a different approach.

 

[Jon W]

Thanks Chris. I think the suggestion from the original bug report to use the master template and conditionals might be the best solution.

 

[Lukas W.]

I'm not sure as what are the reasons behind this stance. If two people work on the same template in different styles, they'll likely end up having issues anyway. Apart from the fact that it's imo highly unlikely that they save at the exact same second, there's a 50% nontheless, that the outdated changes will go unnoticed as long as the person that edits the child saves after the person that edits the parent. I don't really understand what 1 more second of "rightful error detection" would make a difference here.

In contrast, it is way more likely that a save of two templates at the same time happens as part of an automatic process. As they normally only take a few split-seconds, many of them can be performed within a second. Thus the pure chance of this occurring is in my eyes way more likely. Our specific issue with having this all in the master template is the sheer size of the code being generated if people run a nearly three digit number of styles (yes they do that...) on a forum with more than a hundred nodes (yes, they do that too...), so we want to factor that out one way or another, preferably by sorting it into the respective style imo. Spoofing the edit date to falsify it only to correct this issue seems weird to me, especially if only necessary cause we want to catch this one-second window possibility of two people saving a template at the same time.