Opened 9 months ago

Closed 8 months ago

#125 closed enhancement (fixed)

Test for Core Trac #5809

Reported by: wonderboymusic Owned by:
Priority: normal Milestone:
Component: Taxonomy Keywords: has-patch
Cc:

Description

Tests setting a term in 2 taxonomies, changing the name of one, and ending up with 2 terms - rather than changing both

Attachments (1)

terms-test.diff (719 bytes) - added by wonderboymusic 9 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 follow-up: ↓ 3   tomauger9 months ago

Nice!

I'm pretty unfamiliar with unit tests, but doesn't it seem like the first test:
$this->assertEquals( $t1['term_id'], $t2['term_id'] );
ought to fail? Isn't that the point of the ticket - that inserting two terms (one a category, the other a tag) with the same name should have different IDs?

The second test seems good to me, though I think I would have gone about it differently - after updating the second term, lookup the first term again and see if it's name has changed (rather than comparing the IDs). I'm not suggesting you change the second test, but consider adding a third?

Without my patch applied, the first test should pass and the second test should fail.
With my patch applied, both tests should pass.

As is stands now, term_taxonomy_id is the primary key for term_id/taxonomy relationship, so since we create the term in the first insert, the second will reuse it. When the term is updated, WP currently changes the name of both terms because it doesn't care that the term_id is in multiple taxonomies (FAIL) - my patch makes a new term so the term_ids shouldn't match in the second assertion.

comment:3 in reply to: ↑ 1   scribu8 months ago

Replying to tomauger:

Isn't that the point of the ticket - that inserting two terms (one a category, the other a tag) with the same name should have different IDs?

No, the point of the ticket is in it's (current) title: "Updating a term in one taxonomy affects the term in every taxonomy"

So, the patch looks good. I'd commit it, but I'm unclear on the policy:

Is it ok to commit unit tests which are correct, but that will fail until an existing patch is applied to Core?

(In [1029]) add test for updating a term shared by two taxonomies. props wonderboymusic for initial patch.

see #125 and #WP5809

(In [1030]) test the names of the updated term, not the term ids. see #125

It should also be tested that the terms still reference the same posts as before the update.

Related: #130

  • Resolution set to fixed
  • Status changed from new to closed

Discussing the tests separate from the patch doesn't seem to be effective. Closing.

Discussion should be continued in #WP5809.

Note: See TracTickets for help on using tickets.