Skip to content

fix: [meta-templates] empty template can't be deleted at equal ver - #238

Open
DocArmoryTech wants to merge 1 commit into
cerebrate-project:developfrom
DocArmoryTech:fix/metatemplate-removable
Open

fix: [meta-templates] empty template can't be deleted at equal ver#238
DocArmoryTech wants to merge 1 commit into
cerebrate-project:developfrom
DocArmoryTech:fix/metatemplate-removable

Conversation

@DocArmoryTech

Copy link
Copy Markdown
Contributor

getStatusForMetaTemplate() computed meta_field_amount and can-be-removed at the very end of the method, after two early returns. One of those fires whenever the stored version is not older than the one on disk:

    if (intval($metaTemplate->version) >= intval($template['version'])) {
        $updateStatus['up-to-date'] = true;
        $updateStatus['conflicts'][] = __('Could not update the template. ...');
        return $updateStatus;          // can-be-removed never set
    }

MetaTemplatesController::delete() then rejects the request:

    if (empty($templateStatus['can-be-removed'])) {
        throw new MethodNotAllowedException(__('This meta-template cannot be removed'));
    }

So a template holding no meta-fields at all could not be deleted whenever the stored and on-disk versions matched -- which is the ordinary state of any template nobody has edited. The error message gives no hint that the version comparison is responsible.

Removability depends on whether values exist, not on version, so the computation is moved above the early returns. The empty($template) branch still forces false, as before.

Practical effect: template authoring regains a reset. Previously createNewTemplate refused an existing UUID+version, update() no-opped unless the on-disk version was higher, and delete was rejected -- leaving no way to iterate on a template without bumping its version.

Note: $updateStatus['to-existing'] is never assigned within this method --
it is set in getUpdateStatusForTemplate() on a different array -- so the
second half of the can-be-removed condition is always true. Left as-is
rather than changed, since making it meaningful would alter behaviour
beyond this fix.

getStatusForMetaTemplate() computed `meta_field_amount` and
`can-be-removed` at the very end of the method, after two early returns.
One of those fires whenever the stored version is not older than the one on
disk:

    if (intval($metaTemplate->version) >= intval($template['version'])) {
        $updateStatus['up-to-date'] = true;
        $updateStatus['conflicts'][] = __('Could not update the template. ...');
        return $updateStatus;          // can-be-removed never set
    }

MetaTemplatesController::delete() then rejects the request:

    if (empty($templateStatus['can-be-removed'])) {
        throw new MethodNotAllowedException(__('This meta-template cannot be removed'));
    }

So a template holding no meta-fields at all could not be deleted whenever
the stored and on-disk versions matched -- which is the ordinary state of
any template nobody has edited. The error message gives no hint that the
version comparison is responsible.

Removability depends on whether values exist, not on version, so the
computation is moved above the early returns. The `empty($template)` branch
still forces false, as before.

Practical effect: template authoring regains a reset. Previously
createNewTemplate refused an existing UUID+version, update() no-opped
unless the on-disk version was higher, and delete was rejected -- leaving
no way to iterate on a template without bumping its version.

> Note `$updateStatus['to-existing']` is never assigned within this method --
> it is set in getUpdateStatusForTemplate() on a different array -- so the
> second half of the can-be-removed condition is always true. Left as-is
> rather than changed, since making it meaningful would alter behaviour
> beyond this fix.
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

Successfully merging this pull request may close these issues.

1 participant