Support from Acquia helps fund testing for Drupal Acquia logo

Comments

natanmoraes created an issue. See original summary.

natanmoraes’s picture

Status: Active » Needs review
FileSize
8 KB
natanmoraes’s picture

NWOM’s picture

This doesn't seem to work on my end. I have Workflow Fields with the patch #2148935: Make module compatible with workflows defined through "Workflow field" module installed. After applying the patch, I updated via update.php and cleared cache.

I attempted to set an optional Description/Body field on a state to "Required". However, it allows me to save the node form without filling out the field.

I also tried the same using a Relation Select field, with the same results.

caminadaf’s picture

Status: Needs review » Needs work

This patch was conflicting with current dev. I re-patched it.

Also, the only thing done in the patch @natanmoraes provided is the interface. The integration with workflow_node was not implemented.

Moving this to "Needs Work".

caminadaf’s picture

Forgot to upload the patch

fox_01’s picture

I have installed latest dev (2015-May-16) + #2148935-31: Make module compatible with workflows defined through "Workflow field" module and get many conflicts while applying the patch #6 from this issue

lklimek’s picture

Status: Needs work » Needs review
FileSize
9.69 KB

I've manually re-rolled the patch from #6 against latest dev + #2148935-31 . To use , first apply [#2148935-31: Make module compatible with workflows defined through "Workflow field" module] and then apply the attached patch.

This patch also includes some logic to make this feature fully working.

Note that final "required" state is a sum of "required" state of the field and required state from workflow_field config.

fox_01’s picture

#8 works after running update.php

Zekvyrin’s picture

Status: Needs review » Needs work

Well.. I've tried the patch for #8 and although it's working for nodes, it's not working for any other entity...

The reason is that there is no code to make the field required apart from the part in node_form alter function.

I'll try to fix that the following days.. but for now, I'm setting it to "Needs work" as it is not working for generic entities.

Also (because we are changing the _workflow_fields_compute_permissions) we should include this change as well:

@@ -724,7 +724,7 @@ function workflow_fields_field_access($op, $field, $entity_type, $entity, $accou
       }
 
       // Check for visible/editable flags.
-      list($visibles, $editables) = _workflow_fields_compute_permissions($sid, $entity->type, $entity, $op, $entity_type);
+      list($visibles, $editables, $requireds) = _workflow_fields_compute_permissions($sid, $entity->type, $entity, $op, $entity_type);
 
       if (isset($visibles[$field['field_name']])) {
         $is_accessible = ($op == 'view') ? $visibles[$field['field_name']] : ($visibles[$field['field_name']] && $editables[$field['field_name']]);
Zekvyrin’s picture

Status: Needs work » Needs review
FileSize
12.52 KB
2.76 KB

So... I've created a patch which works for me for another entity (entityform).

I tested it and it was working for me, but I haven't done any extensive test yet and there are some grey areas.

Basically in my patch I've added my note from #10 & also implemented workflow_fields_field_attach_form with code similar to hook_field_access which sets required to TRUE if the field is marked as required for this user.

I would like to change it even earlier if possible (for example alter field's instance data before the form element is created), but I couldn't find a way and I don't know if there is any.

I still have 2 concerns though:
1) I'm not 100% sure that this hook is the correct approach. I've also considered hook_field_widget_form_alter as well (and I can also provide a working patch), which is called earlier, but this is called separately in every field. Using hook_field_attach_form allowed me to fetch for data if field is required only once per entity (not once per field) which seemed better.

2) There are some more complex fields (like addressfield) which contain other fields that this approach is not working for them. I don't know if there is a generic way to deal with those (searched but couldn't find anything), or we have to implement something specifically for each one if needed (maybe a different patch).

Thoughts people?

Zekvyrin’s picture

Small fixes (php notices)

Zekvyrin’s picture

And another small correction (in the hook_field_attach_form again)..

Zekvyrin’s picture

Just another update in hook_field_attach_form... I found a bug in my logic (how to make a field required), so I decided to "copy" the logic from hook_node_form_alter for uniformity.

Zekvyrin’s picture

Removed a forgotten dsm from #14..