Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xen created an issue. See original summary.

Xen’s picture

Status: Active » Needs review
FileSize
844 bytes

Attached patch fixes around this issue.

jonathan1055’s picture

Title: Validation interferes with deletion » Validation on unpublish date in the past interferes with node deletion

Hi Xen,
Thanks very much for noticing this, and for the patch. It's odd because I'm sure we did some work on bypassing the validation when deleting a node.

We will have to check that this problem is fixed in our 8.x development first, so I might add a test (in 7.x and 8.x) to cover this case, then it cannot get forgotten.

Jonathan

Xen’s picture

A patch for those that need to patch 1.2.

Status: Needs review » Needs work

The last submitted patch, 4: validation_interferes-1.2-2627370-4.patch, failed testing.

The last submitted patch, 4: validation_interferes-1.2-2627370-4.patch, failed testing.

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
5.99 KB

I've checked 7.x and yes, this is definitely a bug. The work we did previously was to make sure nodes could be deleted when the 'required' field options were set, but we did nothing about dates in the past.

At 8.x the code works differently, and the 'delete' button is now a link which works fine, so there is no problem at 8.x. However, I have expanded the test coverage at 8.x to make sure this does not regress. When this patch passes, I'll commit the tests at 8.x then add tests and patch at 7.x

  • jonathan1055 committed dd9ed9b on 8.x-1.x
    Issue #2627370 by jonathan1055: Expanded 8.x tests for deleting nodes...
jonathan1055’s picture

Now that the 8.x tests are commited, here is the expanded 7.x test. This patch should fail the deletion tests, as the code fix is not included here.

Status: Needs review » Needs work

The last submitted patch, 9: 2627370_9.delete_node_with_past_date.7x.patch, failed testing.

jonathan1055’s picture

Status: Needs work » Needs review
FileSize
4.79 KB

As expected, #9 failed the new tests. Now the tests plus code fix. This should pass.

  • jonathan1055 committed 13d5728 on 7.x-1.x authored by Xen
    Issue #2627370 by jonathan1055, Xen: Validation on unpublish date in the...
jonathan1055’s picture

Title: Validation on unpublish date in the past interferes with node deletion » Validation on publish/unpublish date in the past interferes with node deletion
Status: Needs review » Fixed

@xen Thank you for finding this and for providing the initial patch.
Committed and fixed.

Jonathan

jonathan1055’s picture

Status: Fixed » Needs review
FileSize
646 bytes

I noticed later that it is not sufficient to just skip Scheduler's own validations when deleting the node. If using Date Pop-up then there can already be errors flagged on the form, which prevent deletion. Here is a small addition which solves this.

  • jonathan1055 committed ba09698 on 7.x-1.x
    Issue #2627370 by jonathan1055: Remove form errors from pop-up on...
jonathan1055’s picture

Status: Needs review » Fixed

As before, this fix is not required for 8.x

  • jonathan1055 committed fdef05b on 7.x-1.x
    Issue #2627370 by jonathan1055: Move common test settings to a shared...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.