Patch: support for update $op in taxonomy_uuid [#1224904]

We noticed while trying to update terms that uuids were not being generated. They would only be deleted when a term was deleted, or inserted on creation of a new term. But we needed to update the taxonomy terms with uuids generated from another drupal install. While troubleshooting, we noticed there was no 'update' $op in the uuid_taxonomy() call.

So I decided to write one. In the process, I tried to make some of the other code within the function a little more readable.

I've included a patch which implements update for taxonomy. It is attached to this issue.

Comment	File	Size	Author
#5	support_for_update_in_taxonomy_uuid-1224904-5.patch	2.68 KB	apotek
#5
#3	support_for_update_in_taxonomy_uuid-1224904-1.patch	1.9 KB	apotek
#3
#1	support_for_update_in_taxonomy_uuid-1224904-1.patch	1.15 KB	apotek
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

apotek CreditAttribution: apotek commented 20 July 2011 at 21:47

Issue tags:

-Needs tests

File	Size
support_for_update_in_taxonomy_uuid-1224904-1.patch	1.15 KB

~~Here's the patch. Thoughts?~~

(Don't use this one. Use the one in comment 3.)

Comment #2

q0rban CreditAttribution: q0rban commented 20 July 2011 at 21:13

Title:	Update $op support for taxonomy_uuid	» Patch: support for update $op in taxonomy_uuid
Issue tags:		+Needs tests

subscribe. +1.

Comment #3

apotek CreditAttribution: apotek commented 20 July 2011 at 21:52

Issue tags:

+Needs tests

File	Size
support_for_update_in_taxonomy_uuid-1224904-1.patch	1.9 KB

q0rban identified some oversights in the original extent of the patch. This is fixed in this version. ~~I'm deleting the previous one to keep broken code to a minimum~~ (can't delete the previously uploaded patch. sorry).

Comment #4

markdorison

he/him

CreditAttribution: markdorison commented 20 July 2011 at 23:11

Subscribing.

Comment #5

apotek CreditAttribution: apotek commented 21 July 2011 at 20:18

File	Size
support_for_update_in_taxonomy_uuid-1224904-5.patch	2.68 KB

Now that I understand the uuid_taxonomy function better, I decided to rewrite it to make it a little more performant and clear.

Here's a better version.

Would love to learn how to write tests for this but also notice that DrupalWebTestCase doesn't have any methods for creating or updating or deleting terms or vocabularies.

Comment #6

apotek CreditAttribution: apotek commented 21 July 2011 at 20:20

Status:

Active

» Needs review

Comment #7

apotek CreditAttribution: apotek commented 25 July 2011 at 18:10

Looks like the tests were passed. Any other thoughts on this?

Comment #8

ericduran CreditAttribution: ericduran commented 26 July 2011 at 17:34

Status:

Needs review

» Reviewed & tested by the community

At 1st I was against this patch, but we can't always assume that all the uuids will be created when the term is 1st inserted.

So this patch makes total sense to me.

We're running this on a production site, without any issues.

Comment #9

mikeryan

he/him

English

Murphysboro, IL, USA

CreditAttribution: mikeryan commented 30 July 2011 at 20:53

I'd skip the break after the return in the first switch:-), but looks good. +1 good for migration...

Thanks.

Comment #10

apotek CreditAttribution: apotek commented 1 August 2011 at 19:30

Thanks for your input, Mike. I go back and forth on the extraneous break; Obviously unnecessary if a return has fired, but it also feels more complete and self-documenting to leave it in there. If the Drupal standard is to not have the break there, I'll remove it, but coder module didn't complain.