All of the taxonomies I'm importing have GUIDs attached to the terms. I want to be able to automatically create a guid field as part of the import process.

The attached patch simply moves existing functionality into two new functions - term_fields_add_field() and term_fields_delete_field(), so it can be got at by other modules more easily.

The signature of term_fields_add_field() could be improved, but I'll leave that to someone else - I wanted to make minimal changes.

CommentFileSizeAuthor
#5 term_fields-refactor-791352-5.patch10.88 KBdotton
adddelete.diff7.61 KBdotton

Comments

b-prod’s picture

b-prod commented
Version: 6.x-1.12 » 6.x-1.x-dev
Status: Active » Needs work

Your patch has a bug.

+++ src/sites/all/modules/contrib/term_fields/term_fields.module	(working copy)
@@ -84,15 +84,7 @@
-    $result = db_query("SELECT * FROM {term_fields} WHERE vid = %d ORDER BY weight", $vid);
...
+    $fields = term_fields_get_fields($vid);

This modification does not work: the result of this function is not the same than the original query. And it is not the subject of current issue, so I will create a new issue apart for this and create a patch myself.

And there are some extra spaces to remove.

+++ src/sites/all/modules/contrib/term_fields/term_fields.admin.inc	(working copy)
@@ -426,12 +416,8 @@
\ No newline at end of file

Could you create a new patch without this modification and work on the dev version?

Powered by Dreditor.

b-prod’s picture

b-prod commented

The code factorization has been ported in the dev version: 791434: Factorize a part of code. Actually it is not the only part of code which requires some review. But since D7 will handle term fields in core, I prefer only work on bugs and some features request, because this module will become obsolete.

Could you use drupal_write_record() instead of db_query (so only 1 line is necessary) in the function term_fields_add_field() and serialize(array()) instead of 'a:0:{}'. The code would look better like that.

dotton’s picture

dotton commented

Thanks for the feedback - I'll submit a new patch against the dev version.

b-prod’s picture

b-prod commented

You should use CSV, because it may take some time for the modified dev version to be up to date in the project page. Currently there is still an old version.

dotton’s picture

dotton commented
StatusFileSize
new term_fields-refactor-791352-5.patch10.88 KB

Ok, second attempt. This patch is against HEAD.

To recap, I've created two new functions term_fields_add_field() and term_fields_delete_field() in term_fields.module so I can call them from other modules.

term_fields_add_field() now uses drupal_write_record(), and term_fields_admin_new_submit() uses serialize(array()).

If there's still a bug there please spell it out for me, as I'm struggling to see it.

b-prod’s picture

b-prod commented

OK, I tried to apply it on DEV version, but it failed. So I need to have a patch which I can apply to the DEV version. If you don't have time for that, I will try to work on that, but currently I am not very available.

kenorb’s picture

kenorb commented

Failing for me as well.

b-prod’s picture

b-prod commented
Version: 6.x-1.x-dev » 6.x-2.x-dev
Status: Needs work » Closed (won't fix)

This does not seem to be an issue for 2.x branch.