I needed FillPDF generated PDFs to be stored in a file field on the node they were generated for as part of a project (feature discussed here previously: https://www.drupal.org/node/1606828).

I have developed a FillPDF field sub-module which provides this functionality, only for nodes at the moment.

Upload the files into your fillpdf/modules folder and enable. You can now add a field of type 'Fill PDF' to any node. In the field settings you can specify:

  • whether the generated PDF will be stored as a public or a private file

In the instance settings you can specify:

  • the default Fill PDF form to use
  • if you want to allow selection of the form at the point of generation
  • if so, which forms should be allowed

Once added to a node, the field will not show anything on the node edit form. When viewing the node it will show a table of generated PDFs (with remove buttons) and a button to generate further PDFs within the cardinality limit.

Module should work with 7.x-2.x-dev and 7.x-1.9.

Any feedback/improvements/testing welcome.

CommentFileSizeAuthor
#57 d7_fillpdf_field_module_53-57_interdiff.txt3.83 KBpancho
#57 d7_fillpdf_field_module_57.patch58.23 KBpancho
#53 d7_fillpdf_field_module_52-53_interdiff.txt282 bytespancho
#53 d7_fillpdf_field_module_53.patch62.16 KBpancho
#52 d7_fillpdf_field_module_51-52_interdiff.txt3.26 KBpancho
#52 d7_fillpdf_field_module_52.patch62.19 KBpancho
#51 Screenshot_51.png14.18 KBpancho
#51 d7_fillpdf_field_module_50-51_interdiff.txt5.87 KBpancho
#51 d7_fillpdf_field_module_51.patch60.9 KBpancho
#50 d7_fillpdf_field_module_49-50_interdiff.txt1.88 KBpancho
#50 d7_fillpdf_field_module_50.patch59.26 KBpancho
#49 d7_fillpdf_field_module_48-49_interdiff.txt580 bytespancho
#49 d7_fillpdf_field_module_49.patch59.01 KBpancho
#48 d7_fillpdf_field_module_47-48_interdiff.txt3.08 KBpancho
#48 d7_fillpdf_field_module_48.patch59 KBpancho
#47 d7_fillpdf_field_module_46-47_interdiff.txt565 bytespancho
#47 d7_fillpdf_field_module_47.patch59.04 KBpancho
#46 d7_fillpdf_field_module_45-46_interdiff.txt11.44 KBpancho
#46 d7_fillpdf_field_module_46.patch59.05 KBpancho
#45 d7_fillpdf_field_module_44-45_interdiff.txt6.32 KBpancho
#45 d7_fillpdf_field_module_45.patch54.98 KBpancho
#44 d7_fillpdf_field_module_43-44_interdiff.txt3.59 KBpancho
#44 d7_fillpdf_field_module_44.patch54.07 KBpancho
#43 d7_fillpdf_field_module_42-43_interdiff.txt1.34 KBpancho
#43 d7_fillpdf_field_module_43.patch54 KBpancho
#42 d7_fillpdf_field_module_41-42_interdiff.txt29.78 KBpancho
#42 d7_fillpdf_field_module_42.patch53.51 KBpancho
#41 d7_fillpdf_field_module_40-41_interdiff.txt2.88 KBpancho
#41 d7_fillpdf_field_module_41.patch47.41 KBpancho
#40 d7_fillpdf_field_module_39-40_interdiff.txt3.95 KBpancho
#40 d7_fillpdf_field_module_40.patch45.71 KBpancho
#39 screenshot_39.png31.94 KBpancho
#39 d7_fillpdf_field_module_39.patch46.66 KBpancho
#39 d7_fillpdf_field_module_38-39_interdiff.txt2.3 KBpancho
#39 d7_fillpdf_field_module_37-38_interdiff.txt8.25 KBpancho
#38 d7_fillpdf_field_module_38.patch45.12 KBpancho
#37 d7_fillpdf_field_module_36-37_interdiff.txt2.18 KBpancho
#37 d7_fillpdf_field_module_37.patch44.21 KBpancho
#36 d7_fillpdf_field_module_35-36_interdiff.txt725 bytespancho
#36 d7_fillpdf_field_module_36.patch44.08 KBpancho
#35 d7_fillpdf_field_module_34-35_interdiff.txt2.95 KBpancho
#35 d7_fillpdf_field_module_35.patch43.98 KBpancho
#34 d7_fillpdf_field_module_33-34_interdiff.txt2.55 KBpancho
#34 d7_fillpdf_field_module_34.patch43.98 KBpancho
#33 d7_fillpdf_field_module_32-33_interdiff.txt1.41 KBpancho
#33 d7_fillpdf_field_module_33.patch43.81 KBpancho
#32 d7_fillpdf_field_module_31-32_interdiff.txt5.88 KBpancho
#32 d7_fillpdf_field_module_32.patch43.79 KBpancho
#31 d7_fillpdf_field_module_30-31_interdiff.txt21.13 KBpancho
#31 d7_fillpdf_field_module_31.patch43.47 KBpancho
#30 d7_fillpdf_field_module_29-30_interdiff.txt3.43 KBpancho
#30 d7_fillpdf_field_module_30.patch40.84 KBpancho
#29 d7_fillpdf_field_module_26-29_interdiff.txt18.34 KBpancho
#29 d7_fillpdf_field_module_29.patch40.32 KBpancho
#27 formatter_settings.png40.33 KBpancho
#27 field_settings.png36.2 KBpancho
#27 node_view_simple_formatter_block.png22.3 KBpancho
#27 node_view_table_formatter.png30.86 KBpancho
#26 d7_fillpdf_field_module_23-26_interdiff.txt3.98 KBpancho
#26 d7_fillpdf_field_module_26.patch36.29 KBpancho
#23 d7_fillpdf_field_module_22-23_interdiff.txt8.6 KBpancho
#23 d7_fillpdf_field_module_23.patch36.2 KBpancho
#22 d7_fillpdf_field_module_15-22_interdiff.txt24.88 KBpancho
#22 d7_fillpdf_field_module_22.patch38.49 KBpancho
#15 d7_fillpdf_field_module_14-15_interdiff.txt5.54 KBpancho
#15 d7_fillpdf_field_module_15.patch25.5 KBpancho
#14 d7_fillpdf_field_module_13-14_interdiff.txt5.16 KBpancho
#14 d7_fillpdf_field_module_14.patch26.04 KBpancho
#13 d7_fillpdf_field_module_12-13_interdiff.txt13.08 KBpancho
#13 d7_fillpdf_field_module_13.patch25.45 KBpancho
#12 d7_fillpdf_field_module_11-12_interdiff.txt7.21 KBpancho
#12 d7_fillpdf_field_module_12.patch22.19 KBpancho
#11 d7_fillpdf_field_module_8-11_interdiff.txt7.22 KBpancho
#11 d7_fillpdf_field_module_11.patch20.57 KBpancho
#8 d7_fillpdf_field_module_6-8_interdiff.txt8.22 KBpancho
#8 d7_fillpdf_field_module_8.patch20.59 KBpancho
#6 d7_fillpdf_field_module_5-6_interdiff.txt2.2 KBpancho
#6 d7_fillpdf_field_module_6.patch21.1 KBpancho
#6 errors3.png9.21 KBpancho
#6 errors2.png19.31 KBpancho
#6 errors.png85.29 KBpancho
#5 d7_fillpdf_field_module.patch21.21 KBpancho
fillpdf_field.zip6.55 KBsteveaps

Comments

wizonesolutions’s picture

This looks fantastic. Thanks for contributing it.

I don't see much that needs to be changed, aside from it being tested. My main comments are:

1) Fill PDF -> FillPDF (I changed the spacing of the module name a while ago since people were invariably calling it that anyway)
2) Watch out for extraneous whitespace at the beginning of lines. Saw a good amount of that while I was looking over the files.

I think this would make a good submodule for FillPDF, and when it gets to the point we commit it, it will obviously have proper attribution to your d.o username :)

wizonesolutions’s picture

Bumping to 8.x-4.x-dev. The submodule in this issue is for 7.x-2.x. My thinking though is to port it conceptually to Drupal 8 first, then backport it or just fix up/incorporate the submodule.

pancho’s picture

Assigned: Unassigned » pancho
Priority: Normal » Major
Issue tags: +Needs backport to D7

I'm very interested in this awesome concept and will work on a D8 port. I think this should be doable in the 8.x-4.x branch, but otherwise in 8.x-5.x. We'll then backport the final implementation to D7.

pancho’s picture

First observations testing the D7 version:

1.)
Both the dropdown to select a FillPDFForm and the checkboxes show empty, unidentifiable entries for FillPdfForms that don't have an admin_title. Looks a bit like the D8 screenshots in #3022485-4: Undefined entity label impedes identifying FillPdfForm.

We should really get #3041029: [PP-2] Slightly less optional 'admin_title' into 7.x-1.x now, and at a later point get #3040776: Autocreate 'admin_title' from metadata and mark it required into 7.x-3.x.

2.)
fillpdf_field already gets right what fillpdf 7.x-1.x gets semi-wrong: it already takes care about #3040903: Offer all available file schemes, but only those, preselecting the site default. Another candidate for backport, once committed in 8.x-4.x.

pancho’s picture

Version: 8.x-4.x-dev » 7.x-1.x-dev
StatusFileSize
new21.21 KB

Let's do it this way:

  1. I'm re-posting the original, unchanged D7 version as a patch against 7.x-1.x.
  2. Next step I'm fixing a few issues with the D7 version.
  3. Then I'll switch back to the 8.x-4.x branch and port the module conceptually to D8.
  4. As soon as the D8 version is RTBC, it gets committed.
  5. Then we'll backport all changes to the D7 version, so it can get committed as well.

I'm starting with 1.), the original, unchanged D7 version as provided by @steveaps in the original post. Here we go.

pancho’s picture

StatusFileSize
new85.29 KB
new19.31 KB
new9.21 KB
new21.1 KB
new2.2 KB

There's been a whole lot of errors, warnings and notices:

errors 1
errors 2
errors 3

but in the end, I had to fix just a few lines to get it all running. A very nice contribution!

Note that I only fixed errors/warnings/notices, some coding style and case style here, so anything beyond that still needs to be done.
To make sure the patch is easy to review in spite of the coding style corrections, I'm adding an "interdiff -wiB", which ignores most whitespace, blank lines and case.

pancho’s picture

Couple of minor things:

This can go:

 /**
  * Gets an array of Fill PDF form IDs.
- *
- * If Fill PDf 7.x-2.x is used then it uses the new admin_title attribute as
- * the value with the fid as the key. If a previous versions is installed, use
- * the fid for both key and value.
  */
 function fillpdf_field_get_fillpdf_forms() {
-  $schema = drupal_get_schema("fillpdf_forms");
   $query = "SELECT admin_title,fid FROM {fillpdf_forms} ORDER BY fid";
-  if(!isset($schema['fields']['admin_title'])) {
-    $query = "SELECT fid FROM {fillpdf_forms} ORDER BY fid";
-  }
[...]

Concatenating two integers without an underscore looks a bit messy:

-        $unique_form_id = "fillpdf_field_remove_file_form_{$entity->nid}{$item['fid']}";
+        $unique_form_id = "fillpdf_field_remove_file_form_{$entity->nid}_{$item['fid']}";

Needs proper replacement:

-  $summary = t('Generate button label: ' . $settings['generate_button_label']);
+  $summary = t('Generate button label: @label', array('@label' => $settings['generate_button_label']));

+ many missing t() calls

pancho’s picture

Status: Needs work » Needs review
StatusFileSize
new20.59 KB
new8.22 KB

Also, this is no good idea: the single allowed form gets deleted, suddenly none would be allowed, but in fact all are allowed.
That's why this otherwise user-friendly pattern was removed from all Core modules.

+  // If no allowed forms are selected, early out.
+  if (empty($allowed_forms)) {
+    return;
+  }
[...]
-      // If no allowed forms are selected, show all forms
-      if(empty($allowed_forms)) {
-        $allowed_forms = $all_forms;
-      }

+ some more code style fixes.

Quite nice now & ready for usability testing!

pancho’s picture

We definitely need to think about user permissions here. Currently every anonymous user who may view published content, may also generate and/or delete PDFs. IMO this needs to be restricted to users with edit permission for the host entity. But we might need to be even more fine-grained.

Basically I'm unsure about the use case for the current implementation. I'm sure it perfectly covers what @steveaps needed for his project. But a FillPDF field may be done in very different ways to cover different use cases. I'm asking myself whether this is one we want to promote, whether this is the way to go.

Is this the 70% use case? Maybe we need to figure out and collect all kinds of use cases first, and then see how these might be best accomplished, before putting more work into porting it to D8.

Please everybody: I need testers and some feedback! :)

pancho’s picture

I'm porting it to D8 anyway. We may then better see if and what needs to be changed/added/brought in line with Core patterns/made configurable.

Use cases:

A1.) Open service
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may view the entity, should also be allowed to create and delete any generated PDF files right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too, together with a 'Delete' button so they can be deleted again. ✅

A2.) Self-service
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create and delete any generated PDF files right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too, together with a 'Delete' button so they can be deleted again.

A3.) Admin removal
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create a populated PDF file right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too. As an admin, I want to be able to delete populated PDFs right from there, too.

B1.) Tickbox on entity form
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create and delete any generated PDF files. On the entity form, there should be a tick box to create a new populated PDF file. On the entity view, the last created FillPdfForm should be displayed as a field and should have a 'Delete' button so they can be deleted again.

B2.) Semiautomatic, privileged user decides on or off
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may create a new entity, should also be allowed to create any generated PDF files right from the entity view. There should be a 'Create' button / tick box to do so. Already created PDFs should be displayed as a field there, too. Whenever an entity is updated, existing PDF files should be updated, too. If the user may delete the entity, a 'Delete' button allows deleting it altogether.

C1.) Fully automatic
I want an associated FillPdf Form to be populated automatically whenever an entity is created or updated. The latest populated PDF file should be displayed as a field on the entity view. Whenever the entity gets updated, the populated PDF should be automatically updated, too, so only the current one remains.

C2.) Fully automatic with revision support
I want an associated FillPdf Form to be populated automatically whenever an entity is created or updated. The latest populated PDF file of a particular entity revision should be displayed as a field on the entity view, possibly together with some metadata. New entity revisions create a new PDF file. However if the entity gets updated without creating a new revision, the populated PDF should be automatically updated, too, so there's a 1:1 relationship between an entity revision and a populated PDF file.

pancho’s picture

Regardless, I refactored some parts of the code a bit, mainly to avoid repetition.

Furthermore, if there's only a single FillPdfForm to choose from, there's no point in giving the user a choice.

Also, if no PDF file is attached and no FillPdfForm allowed to create a new one, we don't want to display anything, not even the label.

There's one more @todo: The module doesn't play nicely with non-node entities, as in some places node is hardcoded as entity.

pancho’s picture

Moved all theming from the formatter level to a dedicated theme function, so it may be overwritten. The files are now rendered with a PDF icon, using theme_file_link(), again, this is unless someone overwrites the theme function. Also, there is a "Simple" formatter (no table, no buttons, just links) now for teasers, views etc. Some CSS styling is still missing, though.

Also moved more logic into fillpdf_field_get_fillpdf_forms(), so this security-relevant aspect is centralized in one place.

pancho’s picture

Here's a slightly larger update that makes our new submodule ready for all kinds of entities.

Successfully tested with the following Core entities: node, user and taxonomy_term.
Not yet tested with Webform, Ubercart or custom entity types, though some generic entity handling code is in place.

We might want to use a hash for creating a unique form ID in theme_fillpdf_field_formatter_table(), just the way Inline Entity Form does, but I've not yet implemented that.

pancho’s picture

Patch #13 introduces a bug, as with the naming of fillpdf_field_entity_load() and fillpdf_field_entity_save() I was unwillingly running into Core's hook_entity_load() and hook_entity_save(), so I'm renaming these two. Also, I moved all hook implementations to the end of the module file.

Finally I copied over Entity API's generic entity_save() code. Didn't want to make Entity API a prerequisite, as we don't know yet whether the FillPDF Field will end up as a submodule or as part of the main module.

pancho’s picture

@wizonesolutions proposed requiring Entity API module, which current patch does. This substantially simplifies CRUD operations. I'm removing the Exception handling for custom entity types but keeping the transaction layer, so to roll everything back if saving fails.

As fillpdf_merge_pdf() doesn't produce any useful output for non-node entities without Entity API's Entity Token submodule being installed, too. I'm therefore requiring this one as well.

I'm also keeping the fillpdf_field_pdf_merge() wrapper around fillpdf_merge_pdf() as I don't trust the new $entity_ids parameter to handle nodes exactly the same as if they were passed in using the legacy $nids parameter. This rather new generic entity handling code basically seemed to work, but still needs some refactoring/testing.

wizonesolutions’s picture

OK, so to comment on your use cases:

Use cases:

Use cases:

A1.) Open service
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may view the entity, should also be allowed to create and delete any generated PDF files right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too, together with a 'Delete' button so they can be deleted again. ✅

A2.) Self-service
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create and delete any generated PDF files right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too, together with a 'Delete' button so they can be deleted again.

A3.) Admin removal
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create a populated PDF file right from the entity view. There should be a 'Create' button to do so. Already created PDFs should be displayed as a field there, too. As an admin, I want to be able to delete populated PDFs right from there, too.

B1.) Tickbox on entity form
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may edit the entity, should also be allowed to create and delete any generated PDF files. On the entity form, there should be a tick box to create a new populated PDF file. On the entity view, the last created FillPdfForm should be displayed as a field and should have a 'Delete' button so they can be deleted again.

B2.) Semiautomatic, privileged user decides on or off
I want users to be able to populate a FillPdf Form right from the entity.
Whoever may create a new entity, should also be allowed to create any generated PDF files right from the entity view. There should be a 'Create' button / tick box to do so. Already created PDFs should be displayed as a field there, too. Whenever an entity is updated, existing PDF files should be updated, too. If the user may delete the entity, a 'Delete' button allows deleting it altogether.

C1.) Fully automatic
I want an associated FillPdf Form to be populated automatically whenever an entity is created or updated. The latest populated PDF file should be displayed as a field on the entity view. Whenever the entity gets updated, the populated PDF should be automatically updated, too, so only the current one remains.

C2.) Fully automatic with revision support
I want an associated FillPdf Form to be populated automatically whenever an entity is created or updated. The latest populated PDF file of a particular entity revision should be displayed as a field on the entity view, possibly together with some metadata. New entity revisions create a new PDF file. However if the entity gets updated without creating a new revision, the populated PDF should be automatically updated, too, so there's a 1:1 relationship between an entity revision and a populated PDF file.

Generally: Permissions should follow FillPDF and entity permissions. So if they are allowed to generate PDFs with the entities they can view, then when they create entities directly, they should also be able to generate the PDF.

I think the "C" group of use cases will be most popular from what I have seen, followed by "B" (especially for admins).

A1: It should be optional to expose these buttons, probably options on the field widget so that they can be changed after creation.

A2: Same comment. We could have a new permission specifically allowing PDF deletion. Entity deletion permissions would override this (because if they can delete the whole entity, they can cause the PDF to be deleted anyway).

A3 (and buttons in general): We should have a "Regenerate" (or similar name) button as well in case anything goes wrong the first time. It can happen.

B1: This should be controlled the same way as the buttons, and probably always available for users with the permission (based on FillPDF permissions for creation/regeneration and entity permissions for deletion).

B2: I think this would happen because of the "A" cases anyway.

C1/C2: Auto-generation probably should be a setting (on by default). It would then happen on entity/webform presave.

pancho’s picture

Thanks for your feedback, Kevin!
It's really helpful to know I'm not the only one who thinks usecase C might be the most important one!

I'm currently underway revamping the module so it more or less covers all of A, B and C usecases. A1/2/3 and C1 are already working. B is trickier than expected, but I'll figure it out and will hopefully post both the next version and a few screenshots/screencasts during the weekend. For now I will leave out C2, which is a good followup feature request.

All in all, this looks like is going to be an awesome tool and a nice showcase of FillPDF's power. :)

wizonesolutions’s picture

Pancho, is the D7 version of this "done"? Or are you going to make changes?

pancho’s picture

Pancho, is the D7 version of this "done"? Or are you going to make changes

Unfortunately not at all... :/

I focused on about everything else the last few weeks, so this will definitely need at least a few days for another working patch.

If we want this to be in our upcoming D7 point release, I might however prioritize it vs. other features and tasks.

wizonesolutions’s picture

No rush.

wizonesolutions’s picture

Status: Needs review » Needs work
pancho’s picture

To get this going again, I'm posting the latest draft version.

Use-cases A (button on view page) and C1 (automatic generation) are more or less working, with some rough edges still to be fixed. Usecase B (widget on edit form) is not yet implemented.

However, as it currently stands, I simply don't like it. It's on the way to becoming kind of a Leatherman which does too much, so is too complex. However, the original usecase A with the button on the view page introduces a weird logic where a field is edited on the view page, so needs our own permission system.

Currently don't know how to go forward with it, but I think we'll have to tear out this or that, and possibly even start afresh.

Developed the D8 version in parallel, so once we're settled with the big questions, we will implement it in D8 and then backport to D7.

pancho’s picture

First step at simplifying. We don't necessarily need all different field instance settings for the different trigger modes:

  • If we're in automatic mode and the sitebuilder chose two FillPDF forms: fine, then we're simply autogenerating two PDFs. :)
  • If we're in view button mode: the formatter allows specifying a default form anyway.
  • The only place where we can't have a default form, if two are available, is the edit widget mode. That's because field widgets don't have their own settings page in D7 Core (unlike in D8 Core). But yeah, I think we can live with it in D7 for the sake of simplicity. Contrib/custom code may do a form alter or add more settings or whatever. And in D8, widget settings are on their own page.
pancho’s picture

What I still don't like about it:

1.)
How the generated PDFs are piling up instead of being updated/replaced. I remember you asked this to be optional, @wizonesolutions, so would we add yet another setting?
In this context, I also don't like how field cardinality is handled. If cardinality is reached, the feature obviously ceases attaching more PDFs (though it keeps generating them in automatic mode). I don't think this makes much sense.
I'd rather take the approach that we're always updating/replacing PDFs of the same type, so previous versions get deleted/unpublished. If the host entity is revisionable and we're supporting revisioning, the old version would remain attached to the old vids, so nothing gets lost, it's just not piling up in the current revision.
The only viable alternative I can think of is locking the field to FIELD_CARDINALITY_UNLIMITED, so at least generation doesn't stop working, but then, the PDFs would still pile up.

2.)
Still not sure about permissions.
However: while it doesn't cover all usecases, we could use sane/secure defaults and leave the rest to contrib/custom code.

For automatic mode, this would be: Whoever was allowed to set up automatic generation for this bundle, may also remove generated PDFs on individual entities of that bundle. For more lenient requirements, they'd need custom code.

Entity deletion permissions would override this (because if they can delete the whole entity, they can cause the PDF to be deleted anyway).

No it wouldn't override. If the sitebuilder opts for automatic generation, they (and custom code) may expect the PDF to be present on every single entity of that bundle. The user may delete the entity altogether, but may not break that assumption.

For the two other trigger modes (view button and edit widget), this would be: Whoever may edit an entity, may also attach or delete a generated PDF. For stricter requirements, revisioning needs to be turned on. For more lenient requirements, they'd need custom code.

3.)
Now if in quite some case, we're not showing a "Remove" button, the table looks very empty. We should probably show the date/time of generation and allow further customizing it via a view, akin to Viewfield, but without the complexity; if possible, without any additional settings: if Views module is installed, a corresponding view will get created, too, and may be further customized. We'd just need to make all relevant metadata available.

4.)
If revisioning is on, do we create a new revision each time a PDF is generated and attached? I'd say: yes.
But what do we do if there's no revisioning, particularly upon removal? Is the PDF file rightaway deleted or is it just detached? In the former case, the button should say "Delete" rather than "Remove". In the latter case, which would be more consistant between revisioned an non-revisioned entities, how does the admin administrate stale PDF files? Or would we keep them attached but "unpublish" them somehow?

5.)
Where do we store the PDF files? Every FillPDF form brings its own storage scheme and destination_path, yet the field settings allow once more to choose a scheme (and in the D8 version) a destination_path inherited from the File field.
Sure, I can imagine use cases where site builders would like to decide on bundle or field level. Having both options might be nice (and comes for free), but is yet another layer of complexity and source of errors which I’d actually like to avoid. Finally, which setting “wins”?
On the other hand, all File fields allow specifying the storage destination. Do we want to deviate from that?
To make this work Usability-wise, we’d need at least an “override FillPDF form defaults” checkbox, and would use FormAPI #states to otherwise hide the superfluous settings.

pancho’s picture

Assigned: pancho » Unassigned
Status: Needs work » Needs review

The field doesn't need its own block, as Field as Block module provides everything for that usecase and works fine with FillPDF field. We should however have a horizontally more compact formatter that still supports creating and deleting PDFs.

Furthermore, we should have an exposed filter for the Content view and a bulk action to (re)generate PDFs.

What else do we need to cover reasonable use cases? Please everybody come up with suggestions!

I'm setting this issue to needs review, so it gets a bit exposure and possibly some review/remarks.

pancho’s picture

Just a minor update to ensure the D7 table schema matches D8 Field API's defaults.

pancho’s picture

And some screenshots:

FillPDF field (table formatter) with generate/remove buttons on the node view page:
Table formatter on node view

FillPDF field (simple formatter) as a block:
Simple formatter block

Field settings:
Field settings

Formatter settings:
Formatter settings

pancho’s picture

A few more thoughts:

1. (Re #24-5)

Where do we store the PDF files? Every FillPDF form brings its own storage scheme and destination_path, yet the field settings allow once more to choose a scheme (and in the D8 version) a destination_path inherited from the File field.

I thought about it again, and tend not to allow overriding the FillPDF settings on an entity/field level. To be consistent, we’d have to allow overriding the file name pattern, too, and then it gets too complicated and seems superfluous in most usecases.
In the future, where all FillPDF files are merged using a FillPDF field, webform element or other UI tools rather than by hitting a path, we might hand control to the fields, elements etc. But for now, it would be too much duplication.

We should make sure there is a way to easily override all settings using custom code, but that should be enough.

2.
In D8, where widgets have their own settings page, we can merge the “automatic mode” into a special case of the “edit widget mode”. On the field settings level, we would configure which FillPDF forms are available. On the widget settings page, we may configure which of them is turned on/off by default or is locked on. The “locked on” ones are automatic, and if there are no others, the widget is altogether hidden.

We can’t really do that in D7 though. It simply wouldn’t work UI-wise, as in D7, any widget settings are merged into the field instance settings page.

3.
We should make sure that custom code using FormAPI #states (or Conditional Fields module) can easily access particular FillPDF forms and turn them on or off. Therefore, even in D7, automatic mode should internally translate to a special case of the “edit widget mode”. Also, we should have #3049646: Use an alphanumeric 'machine_name' primary ID for FillPDF forms, not as a strict prerequisite but as a followup.

4.
At least on the widget, but optionally as well in the table formatter, we’d have to show the ‘admin_title’ which would become a real label then. #3040776: Autocreate 'admin_title' from metadata and mark it required is no hard prerequisite either, but would be helpful. We might want to rename the field into ‘title’ then, so to not indicate it would remain hidden from users.

pancho’s picture

Worked on the missing usecase B (see list in #10): generating PDF on the edit page.

It is somewhat working now. However, I didn't find a way to have the value of an arbitrary FormAPI checkbox available in Field API fillpdf_field_entity_update(), as it obviously wouldn't get stored with the entity. An extra field wouldn't work either. So instead, I added a "Generate PDF" button, which however isn't a viable solution, as the PDF may only be generated after the entity is saved.
Also, there's an issue with the wrong button being triggered.

The other two modes / usecases are working quite fine.

Further changes:

  • added metadata (filesize, creation date, owner) to the table formatter
  • added views relationship support.
  • removed URI scheme setting
  • locked the field at unlimited cardinality
  • this and that...

@todos:

  1. Find a way replacing the buttons with checkboxes on the edit page, and somehow getting the value through to hook_entity_update().
  2. Most formatter level settings may have to move to the field instance, if they are used by the edit page widget, too.
  3. Do away with all the permission settings, see #24-2.
  4. Re #24-1: Still think updating/replacing would be better. In automatic mode, we we would just replace all files. In view button mode, we would add an "Update" button and would show the "Generate" button only until there's a PDF file of each type. How do we figure that out though? May we simply choose arbitrary $delta keys with the FillPdfForm ID, or do we add another property to the field schema?
  5. Quite some more, see all the other comments...
pancho’s picture

More robust sample link generation on the field settings page.

pancho’s picture

With some input by @wizonesolutions, I figured out most of the outstanding issues!

Re #24-1/29-4, we decided there is no use in collecting outdated versions of the PDF file in the current revision. If people want old versions to be archived, they should turn revisions on. In the current revision, each type of PDF form gets updated/replaced now.
Revisioning works, too.

Re #29-1: The checkboxes on the edit page triggering PDF (re)generation, are working now, too.

So out of the usecases in #10, we're now supporting almost everything, except for the semiautomatic mode (= allow the user to configure automation per entity) or usecase B2. Remembering which PDF files should be re/generated per entity would require yet another field property, which is no major problem, but should IMO be a separate feature request.

Anyway, for UX we're tending to merge all currently separate "modes" into one. Automatic mode would then be just a special case of edit widget mode, with the checkboxes being invisible. This would also allow un/setting checkboxes based on other input / data in a form_alter().

Also refactored the PDF generation code, so for multiple fields x multiple instances, most code only runs once.

There are quite some more UX and DX @todos, but basically this should be ready for testing.

pancho’s picture

Depending on node type settings, autocreate a new revision as well, when attaching a PDF generated by hitting a button on the view page.

pancho’s picture

Fixed available FillPDF forms in edit widget.

pancho’s picture

This patch programmatically integrates automatic mode into the edit widget mode. Doing so allows form_altering the visually hidden checkboxes in automatic mode.

The distinction on the UI however remains in place. Automatic mode essentially means all FillPDF forms marked available in the settings are checked and locked (actually just hidden). We can merge the two modes in the UI, but then we need to select a default on/off value and a locked property to each available FillPDF form. While slightly more powerful, I'm unsure that would be easier to use. But let's see...

pancho’s picture

Make sure hook_entity_update() resp. hook_entity_insert() only run if hook_field_attach_submit() set FillPDF forms to populate.
Remove the now redundant $entity->fillpdf_op flag. The new $entity->fillpdf_forms approach is better.

pancho’s picture

Fix D7 Core content translation.
D8-style entity translation will follow, once FillPDF field landed in D7, was ported to D8 and translation handling backported to D7.

pancho’s picture

More fixes regarding D7 Core content translation.

pancho’s picture

StatusFileSize
new45.12 KB

In the table formatter, add a "Regenerate" button to each existing PDF, so the generic "Generate PDF" button is only shown for additional FillPdfForms that are available but not yet populated.

pancho’s picture

Turn operations into a ctools dropbutton:
Screenshot

pancho’s picture

Simplify extracting FillPDF form IDs to re/generate. We don't have to loop through the fields twice.

pancho’s picture

Let's provide our own replacement for the behemoth fillpdf_merge_pdf() with all its legacy baggage.

pancho’s picture

  • Add docblocks documenting functions and hook implementations.
  • Avoid using $fid or $form_id: $fid is ambiguous in regard to managed files and FillPDF forms. $form_id is ambiguous in regard to FillPDF forms and Drupal forms.
  • #3052143: Replace "<br />" by "\r\n"
  • Fold fillpdf_field_read_fields() into the only function actually using it, fillpdf_field_field_attach_submit().
  • Some more code refactoring.
pancho’s picture

Use file_file_download() for our hook_file_download() implementation, just the way image module does.

pancho’s picture

Some more minor code cleanup.

pancho’s picture

Merge in our own version of fillpdf_action_save_to_file() that doesn't create fillpdf_file_context records but registers the host entity with file_usage_add().
Also, renamed fillpdf_field_generate_pdf() to fillpdf_field_attach_pdfs().

@todo: Private file access check needs to be adapted.

pancho’s picture

Status: Needs review » Needs work
StatusFileSize
new59.05 KB
new11.44 KB

To get private file downloads working again, unfortunately we have to do a couple of things here:

  • Fix fillpdf_file_download() which overreaches and blocks access for files it hasn't registered at all. (fixed. Note that this is the first and only change needed in the parent FillPDF module.)
  • Switch back to using the ambiguous 'fid' identifier instead of D8-style 'target_id', as file_get_file_references() unfortunately hardcodes the column name. (fixed)
  • Make sure we're rolling all columns of the managed file into the field, when loading an entity. Is that hook_field_load()? It however doesn't work yet.
pancho’s picture

Status: Needs work » Needs review
StatusFileSize
new59.04 KB
new565 bytes

Oh, this is the trick: #702586: hook_field_load() and friends are not real hooks

So it's FIELD_NAME_field_load() not MODULE_NAME_field load(). Accordingly, with the field name being just 'fillpdf', we need to implement fillpdf_field_load() not fillpdf_field_field_load().

pancho’s picture

Now that we're loading the whole file entity into the host entity, we can avoid some additional file_load() calls.

pancho’s picture

#46 worked, I just missed flushing the cache, as it's a callback rather than a real hook.
So #47 was bull$#!+
#48 makes sense, but only in combination with reverting #47, so here's a new one.
Private file downloads should now be working fine again.

pancho’s picture

Minor nitpicks.

pancho’s picture

Major UX improvement: in edit widget mode, merge table with checkboxes to a custom tableselect element.
This is how it looks like now:
screenshot

Also unhide description field on the settings page - it is now always shown on the widget, if present, and needs to be customized.

pancho’s picture

For those who don't want a big table displayed on the edit form, I'm adding a 'fillpdf_simple' widget that doesn't show more than a minimum.

pancho’s picture

We're integrating with Views but currently aren't using Views to provide our own functionality. Allowing to customize our tables and tableselects would be awesome, but particularly for tableselects we'd need a custom views field handler, so should be a followup.

Also, while we're currently using Entity API, we don't necessarily need Entity tokens. As with our parent module FillPDF, if using non-node entities, you won't be able to do much without Entity tokens. But that doesn't mean we have to require it.

CTools we're using for dropbuttons, so this one has to stay in for now.

pancho’s picture

Assigned: Unassigned » wizonesolutions

At this point, permissions and formatter configuration remain @todos. But apart from that, this should be ready for review.

Assigning to @wizonesolutions for some code review and manual testing on 7.x-1.x.

wizonesolutions’s picture

Assigned: wizonesolutions » Unassigned
Issue tags: +Needs manual testing/review

I'm just tagging this but unassigning, as Liam could also do this review. I don't have time to do it right now. I will try next week or during Drupal Dev Days if I have some time.

liam morland’s picture

Status: Needs review » Postponed
pancho’s picture

Plain reroll without patch from #3052463: fillpdf_file_download() overreaches. Should work once that one landed.

liam morland’s picture

Assigned: Unassigned » pancho
Status: Postponed » Needs review
pancho’s picture

Version: 7.x-1.x-dev » 8.x-4.x-dev
Status: Needs review » Patch (to be ported)

I’m porting it to D8 now. We can still backport any changes to the D7 patch.

avpaderno’s picture

Issue tags: -Needs manual testing/review +Needs manual testing