Right now the module can clone fields from 1 bundle to others.

I would like to add a submenu that would clone fields from various bundles into 1 bundle.
I working on a patch for this by copy functionality from http://drupal.org/project/field_copier

Let me know if you are already working on this.

CommentFileSizeAuthor
#7 field_tools-add_clone_to_bundle-1604704-7.patch13.85 KBtedbow
#4 field_tools-add_clone_to_bundle-1604704-4.patch14.78 KBtedbow
#3 field_tools-add_clone_to_bundle-1604704-3.patch13.24 KBtedbow
#1 field_tools-add_clone_to_bundle-1604704-1.patch12.5 KBtedbow

Comments

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Active » Needs review
StatusFileSize
new field_tools-add_clone_to_bundle-1604704-1.patch12.5 KB

Ok added a patch for this

joachim’s picture

joachim commented
Status: Needs review » Needs work

Looks like a nifty feature.

Can you run your changes through Coder for whitespace and formatting issues?

Also, it seems to overlap with your patch at #1604768: Check that fields aren't restricted to certain entity types . That's fine, of course, but we need to be aware of that when committing them :)

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new field_tools-add_clone_to_bundle-1604704-3.patch13.24 KB

Ok, ran through coder and reattached.

Also fixed some labeling issues.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new field_tools-add_clone_to_bundle-1604704-4.patch14.78 KB

ok new patch(ran through coder).

I changed the submit functions for the other 2 clone forms to call _field_tools_add_instance_to_bundles

I figure there should be 1 central function for adding instances. This function does checking to make sure a field can be attached to an entity type and also if it is already attached.

Also it displays messages to the user about fields that were skipped b/c of these rules and messages for fields that get attached.

Let me know what you think.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

Just to note the only warnings I get from Coder on this now are:

Line 3: @file description should be on the following line (Drupal Docs)
* @file field_tools.module

I figure I won't mess with this b/c not related to this issue.

And

Line 231: Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
'#title' => $entity_type['label'],

I don't think this is an issue b/c $entity_type['label'] is coming straight from "entity_get_info()".

joachim’s picture

joachim commented
Status: Needs review » Needs work

Argh, this isn't applying any more. Possibly because I did a whitespace fix... :/

> I figure there should be 1 central function for adding instances. This function does checking to make sure a field can be attached to an entity type and also if it is already attached.

Yes, definitely!

+++ b/field_tools.admin.inc
@@ -214,4 +196,170 @@ function field_tools_options_entity_bundle() {
+/**
+ * @param array $instance
+ *   field instance to be added
+ * @param array $new_instances
+ *   keys are entity_type machine names, values are arrays of bundle_names
+ */

This is missing a one line description, something along the lines of 'Helper to add field instances to multiple bundles.'

Then each param description needs to be a full sentence at least.

'An array describing entity bundles on which to create field instances. Each key is an entity type machine name, each value is an array of bundle machine names of that entity.'

Can you also make sure there's one blank line between each function? Again, coder review should point that sort of thing out.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new field_tools-add_clone_to_bundle-1604704-7.patch13.85 KB

Ok remade patch against latest git changes

This is missing a one line description, something along the lines of 'Helper to add field instances to multiple bundles.'

Then each param description needs to be a full sentence at least.

'An array describing entity bundles on which to create field instances. Each key is an entity type machine name, each value is an array of bundle machine names of that entity.'

Ok made these changes

Can you also make sure there's one blank line between each function? Again, coder review should point that sort of thing out.

Made those changes. But I tested coder review and it wasn't catching this.

joachim’s picture

joachim commented
Status: Needs review » Fixed

Hmm that's weird. You might need to set Coder to be more strict -- I spotted a few more problems with spacing in docblocks and length of lines. Anyway, enough patch tennis -- I've made the fixes and committed it.

There was a missing t() too, which I wouldn't expect Coder to catch. Also, I've broken up replacement variables in t() calls to being on their own line, but that's just my preference :)

- #1604704 by tedbow: Added form to clone fields from various bundles into the current bundle.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

Hmm that's weird. You might need to set Coder to be more strict -- I spotted a few more problems with spacing in docblocks and length of lines. Anyway, enough patch tennis -- I've made the fixes and committed it.

I will look into it. I have coder set to "minor (most)" warnings. I only got the errors mentioned in #5 above.

Thanks for committing it.

Status: Fixed » Closed (fixed)

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