Problem/Motivation

When adding a File Download Product Feature, the Product Feature ID (pfid) is not available in the beginning of the process (as expected). The 'pfid' gets created by uc_product_feature_save() during the execution of uc_file_feature_form_submit().

The issue is that there is an index in the $form_state called $form_state['values']['pfid'] which is expected to store the 'pfid' but is NULL even after we get the 'pfid' from uc_product_feature_save().
This makes it hard for other modules to rely on the $form_state data whenever altering this form and adding another submit handler because the 'pfid' would only be available in the $form_state whenever the user goes back and edit the Product Feature.

Proposed resolution

The solution is to set the 'pfid' on the $form_state as soon as it is made available by uc_product_feature_save().

CommentFileSizeAuthor
#2 ubercart-missing-pfid-2909260-2-D7.patch555 bytesdscl
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dscl created an issue. See original summary.

dscl’s picture

dscl CreditAttribution: dscl at DevBrains commented
Title: Product Feature ID is not available on form_state after form is submitted » File Download Product Feature ID is not available on form_state after File Download add form is submitted
Status: Active » Needs review
Issue tags: +uc_file
FileSize
ubercart-missing-pfid-2909260-2-D7.patch555 bytes
TR’s picture

TR CreditAttribution: TR commented
Category: Bug report » Feature request
Issue tags: -Product features, -file download, -uc_file
Morbus Iff’s picture

Morbus Iff
they/them
CreditAttribution: Morbus Iff commented
Category: Feature request » Bug report

Seems like a bug to me, as it affects other code in uc_file_feature_form_submit():

If this is a new file being created, then PFID is empty here:

  $file_product = array(
    'fid'         => $file->fid,
    'filename'    => $file->filename,
    'pfid'        => $form_state['values']['pfid'],

which causes it to remain empty here:

 $data = array(
    'pfid' => $file_product['pfid'],

Then, we create the feature, which sets the pfid into $data via reference:

  $form_state['redirect'] = uc_product_feature_save($data);

but this newly-generated pfid is never saved back into $file_product, which is then checked later:

  // Insert or update uc_file_product table.
  $key = array();
  if ($fpid = _uc_file_get_fpid($file_product['pfid'])) {
TR’s picture

TR CreditAttribution: TR commented

So please clarify, what doesn't currently work?

I will be glad to add something to improve extensibility (feature), but a bug means something isn't currently working properly...

  • TR committed be69f9d on 7.x-3.x authored by dscl 
    Issue #2909260 by dscl: File Download Product Feature ID is not...
TR’s picture

TR CreditAttribution: TR commented
Category: Bug report » Feature request
Status: Needs review » Fixed

Thanks. I committed the patch.

TR’s picture

TR CreditAttribution: TR commented
Version: 7.x-3.x-dev » 8.x-4.x-dev
Status: Fixed » Patch (to be ported)

Actually, need to put this in D8 too ...

  • TR committed e653275 on 7.x-3.x 
    Issue #2909260 by TR: Role grant Feature ID is not available on...
TR’s picture

TR CreditAttribution: TR commented

I made a very similar change to uc_roles.module, since that is the other product feature module in Ubercart, and it shares much of the same code structure.

Both uc_file and uc_role need this patch in D8 now.