After updating Drupal to 4.7.3 in flexinode administration page (/admin/node/types) link 'Add field' works incorrectly.

To reproduce:
- install core drupal to 4.7.3
- install flexinode.module from 4.7 branch
- add new content type in /admin/node/types
- click add for any field type
: Incorrect form shows (it missing hidden edit[field_type])

Reason:
In following code,

      // load the field select form up into $form_values. formapi is teh r0x0rZ.
      flexinode_field_select();
      $output = flexinode_field_form($form_values['field_type'], $form_values['ctype_id']);

Global variable $form_values now stored differently. So in patch I just replaced this code by

      $output = flexinode_field_form($_REQUEST['edit']['field_type'], $_REQUEST['edit']['ctype_id']);

and it works. But I think, there is need to rewrite some logic to use standard form API validation schema.

CommentFileSizeAuthor
#21 flexinode.diff_1.txt1.96 KBradev
#18 flexinode_5.module36.2 KBradev
#17 flexinode.diff_0.txt38.03 KBradev
flexinode.diff.txt625 bytesradev

Comments

t.a. barnhart’s picture

t.a. barnhart commented

i've reported this in http://drupal.org/node/77182

i don't know if the problem is the admin page or not, but i do know flexinode is bust in 4.7.3. and no one has been assigned the issue, which means upgrading to 4.7.3 ain't gonna happen.

did no one test flexinode first?

radev’s picture

radev commented
Assigned: Unassigned » radev

Flexinode broken due to some incorrect usage of form API.

If you want - I can attach my fixed version of flexinode.module for testing.

Also, I`m assign this to me, but I can`t commit patch to CVS.
Please someone review this ASAP.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented
Status: Needs review » Reviewed & tested by the community

After updating to Drupal 4.7.3, whenever I added a field to a content type, no content from any field would display for that type, and all fields disappeared from the content submission form. This patch has fixed the problem for me.

morbus iff’s picture

morbus iff
they/them
commented

I don't believe (nor do I /want/ to believe) that using $_REQUEST is the answer.

morbus iff’s picture

morbus iff
they/them
commented
Status: Reviewed & tested by the community » Needs work
Bèr Kessels’s picture

Bèr Kessels commented

I am unable to reproduce this.
Are you certain it is not due to some old broken field.
Did this happen between the release of drupal 4.7.2 and 4.7.3?

Using superglobal request is not the solution, as Morbus points out. please look at the function arg(), or else just pass more/better function arguments along.

radev’s picture

radev commented

How to reproduce bug:
1) Clean install of Drupal 4.7.3 version (not 4.7.2)
2) Install Flexinode from 4.7 CVS branch.
3) Add new flexinode content type.
4) Try to add any field - in field editing form any settings related to content type is missing.
5) After that - fields do not working properly - node add form is empty for created content type.

Current approach in flexinode module is using global variable $form_values from drupal_get_form() function. But because some changes in 4.7.3 this variable don`t work now as expected my module author.

I know that $_REQUEST is UGLY solution, but it works for this time.

Using arg() is not suitable, because it can`t parse POST variables (we retrieving field type from POST request).
Another solution will be rewrite content type add form in such way - that it will convert field type to some path args. Or use form API for validation. But this will take a few more days because I have little free time to spare on this.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented

I appreciate the attention given to this issue. In response to #6, yes this did happen between 4.7.2 and 4.7.3. According to Michael Cheng's comment in issue 77182 (which so far has no solution), Flexinode works as it used to if lines 63, 66, and 168 of includes/file.inc are commented out.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented
Title: After update to Drupal 4.7.3 flexinode admin page broken » Adding fields breaks Flexinode in 4.7.3
Stol’s picture

Stol commented

I didn't need the patch to show flexinode fields. I observed that old content types did show flexinode fields on the submit form while newly added content types didn't.

So I checked the flexinode_fields table in the database and I observed that the cells of the column field_type of the new content types were empty. I added manually the type ('file' in my case) and the submit form of flexinode works correct again. But submitting itself don't work correct yet.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented

My comment #8 contains an error. It is includes/form.inc that has changed in Drupal 4.73. The lines that Michael Cheng suggested commenting out were added to fix a bug described in issue 63990 that affected the comment module. I haven't had time to study how this change affected Flexinode, but I did notice that a change had to be made to the comment module as well. Anyone who would like to study the change can look at CVS commit 35451.

spjsche’s picture

spjsche commented

Hi,

I would just like to concur that upgrading to 4.7.3 with flexinode, I am unable to add any fields to a new content type that is created.

Is there any estimate on a fix for this issue, or do I revert back to 4.7.2.

radev’s picture

radev commented

I will try to fix it before 21 August...

spjsche’s picture

spjsche commented

Brilliant, you are a star.

Thanking you for your efforts.

Bèr Kessels’s picture

Bèr Kessels commented

Is it correct to say this is a problem with the file field? If so: why do you think file-fields for flexinode were removed from 4,7 :) They are broken!

Bèr

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented

No, we have been complaining that none of the fields we have added after the 4.7.3 upgrade work.

radev’s picture

radev commented
StatusFileSize
new flexinode.diff_0.txt38.03 KB

This is new version of patch. It is not changing urls Flexinode uses in admin area.
Just using already used code - is gets Field_type from POST request. It was used before for ctype_id, I just added proper handling of field_type in same way.

Correct implementation without using $_POST will require rewrite of major module parts. Perhaps, for 4.8 I will do such rewrite, may be in cooperation with Bèr (on mail-list was some post about this subject).

So, for now I think this is last version of patch. Other solution is to rewrite Flexinode for 4.7 from ground up - which I wouldn`t do.

radev’s picture

radev commented
Status: Needs work » Needs review
StatusFileSize
new flexinode_5.module36.2 KB

Attaching patched version of module.

Actually using it on some sites upgraded to 4.7.3

If it is not satisfying solution for 4.7 branch - please make it postponed or call someone else to fix all places in flexinode.module where parameters was read from global arrays.

Bèr Kessels’s picture

Bèr Kessels commented

Radev, just a comment, your patch looks weird, it contains the whole module. In future please follow the guidelines in http://drupal.org/diffandpatch, for now leave the patch as it is, it works now, it is just very hard to read.

The patch and solution looks good, using $POST is not that bad, especially since we use it all over the place in flexinode anyway. I referred to the $REQUEST wich was not a good solution IMO.

I want at least another confirmation from someone that this patch solved the problem, since I could not reproduce it in the first place :) Then i'll commit.

Thanks for the hard work, radev

TSE’s picture

TSE commented

yes it works fine for me too ;)

had the same bug 4.7.3 with flexinode and was not able to create any new types or add new fields to existing. now its all fine using the patch flexinode_5.module.

thanx a lot.

TSE

radev’s picture

radev commented
StatusFileSize
new flexinode.diff_1.txt1.96 KB

Sorry, Cervisia was configured to include 65536 lines of context... My fault.
Attaching corrected version of patch - may be someone whould review it.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented

Patch works great. I can now add fields to existing content types.

Bèr Kessels’s picture

Bèr Kessels commented

Thank you all for the reviews and tests!

Rade, thanks and lots of kudos for the hard work on the patch.

I have committed it to 4.7, but not to HEAD. in HEAD we need to look at a proper solution for the underlying issue: flexinode sucks with formapi.

Bèr Kessels’s picture

Bèr Kessels commented
Status: Needs review » Fixed
spjsche’s picture

spjsche commented
Category: bug » support

Just to say that I too can add fields to my content types, and also create and submit the forms, but unfortunately, although the data is in the database, it does not get displayed.

Am I missing something ?

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented
Category: support » bug

Try submitting a new support request. Issues get unmanageable when they are used for multiple purposes.

Stol’s picture

Stol commented

Patch doesnt' work for me. Submitted values of custom (required) fields are seen as empty. I'm on IIS 5, maybe a windows thing.

radev’s picture

radev commented

@Stol
Can you open another issue?
And describe it with more detail on required steps to reproduce. I will look into that...

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)