I've tried to update Legal.module for 4.7 with some limited success. Displaying the admin form hasn't been too difficult but there is a bug that I just cannot find.

Basically what happens is that if you go to /admin/settings/legal and attempt to add some terms and conditions, you will always get the error message Terms & Conditions must be entered. whenever you click Preview or Save.

I don't know enough to fix this or figure out where the problem is but I've attached my modified file if Robert or any other interested (desperate!) user wants to have a go at fixing it.

Regards

Patrick

Comments

robert castelo’s picture

I've dived into the deep end of 4.7 module updates by doing the user_import.module first. I've got quite a few modules to update, but as I think the Drupal 4.7 release is still a month or two away, I should be able to have them ready by the time 4.7 comes out.

Not sure how soon I'm going to get round to updating this one, but if anyone needs it urgently get in contact to sponsor the update, and I'll move it up my to do list.

Patrick Nelson’s picture

Title: Attempted update for 4.7 » Ready for 4.7
StatusFileSize
new9.22 KB

I've had another go and the module seems to work fine in 4.7 now for anyone that wants it.

Unfortunately, I don't have CVS access nor can I make a patch, so the complete module is attached.

Regards

Patrick

rbrooks00’s picture

This module is critical to our site so I'll download this module, test it and submit a patch in this issue. Hopefully I can get that done tonight.

Patrick Nelson’s picture

Sorry about this, but this patch actually does NOT work. I will try and investigate and work out why.

Any input would be helpful.

a39wk1lkdmn’s picture

Tried your module out against cvs, and yep, no Terms and Conditions on the /legal page (only the title). However, here's an error message that turns up in the logs during a new user registration that might be a clue:

Location: /user/register
Message: Invalid argument supplied for foreach() in
includes form.inc on line 304.
Severity: error

When the legal module is enabled, the above error occurs and the user attempting to register gets a white screen. When legal is disabled, registration works OK.

Hope it helps,
Randy

jaxxed’s picture

Title: Ready for 4.7 » This one is : Ready for 4.7
Assigned: Unassigned » jaxxed
Category: bug » feature
StatusFileSize
new4.64 KB

I am now using this module (patched) with Drupal 4.7 rc3

I have slightly modified the module for my needs, in order to slightly change the way the terms are displayed on the /legal page.
Additionally I have separated the form generation and the form theming. One thing that could be objected to is that I have put static text displaying into a form (just to keep the entire system valid for the drupal form mechanisms.)

In the rar is the legal.module file and a patch for April 24 cvs copy of the legal.module file. The patch is a lot of changes just because I wasn't trying to minimize change of the code (elimination of white-space etc,) but in general the changes are limited to display and the form generation (focusing on the registraiton response.)
Please note that I am using HTML TOS, which I am displaying a straight up in html on the legal page. This makes displaying the content in a text area useless for me as this html would stay in place. I have found that the css div options works well in all browsers and am considering dropping the text area feature, but will also considering forcing the /legal page to use a text area if so declared for the reigstration page if the developer or other people like this patch and have the need.

Jax

jaxxed’s picture

Title: This one is : Ready for 4.7 » This one is : Ready for 4.7 -- CONTINUED
StatusFileSize
new4.64 KB

a silly html error (on my part) cut off most of my comment on the patch I just submitted, so I am writing again:

This rar I included has a new legal.module and a diff patch for the as of Apr 24 cvs copy of the legal.module.

The differences are cosmentic (reduction of whitespace and indentation changes) ,functional (separation of form generation from theming of the form), and patch (making the user hook return a drupal form array - which it wants despite the drupaldocs manual for the hooks explicit declaration.)

Please note that I have made some changes for myself that mess with the use of a textarea+plaintext TOS, because I am forcing the /legal/ menu page to display the TOS as a long html string, not a scrolling box. I would consider fixing this if the developer or other people use my patch and desire that change.

If anybody wants to know more about the error I can forward them the message I sent to the developer describing how to fix it. If anybody fixes my patch or properly formats/documents it - I'd appreciate seeing their results so that I can include their patch in mine.

AjK’s picture

After patching I got this in error_log

[Mon May 1 13:55:01 2006] [error] PHP Parse error: parse error, unexpected '?' in /home/drupal/contributions/modules/legal/legal.module on line 120

I changed line 120 from

'#default_value' => ?$conditions['display']?$conditions['display']:$edit['display'],

to

'#default_value' => ($conditions['display'])?$conditions['display']:$edit['display'],

and at least the error went away and the admin page now displays. Just tried it out and it appeared to work as expected. However, test for yourselves.

--AjK

Walt Esquivel’s picture

Hello,

Does anyone know if the legal.module will be updated for 4.7 anytime soon? Is it the FormsAPI update that's the holdup or is it something else?

http://drupal.org/node/59396 states...

legal - needs FormsAPI update

Thank you,
Walt

robert castelo’s picture

I'll be updating it sometime this month, just a question of when my workload will allow me to do it.

rbrooks00’s picture

StatusFileSize
new9.56 KB

Based on the comments from AjK I updated the code provided by jaxxed and tested it out on my site. After that everything seemed to run just fine. I'm using 4.7 (final) and the HTML scrollbox version. New patch is attached for testing.

SavoryMedia’s picture

After I finally figured out which file to patch with what patch (ie. the latest patch and the original legal.module file from CVS), I got the patch set up and the module installed on my 4.7 prototype site...set up the T&C with the CSS scrollbox version...and everything works like a charm. No bugs anywhere....yet. ;)

Good work and thanks!

Walt Esquivel’s picture

I would love to test the legal module but, alas, I'm not savvy enough to figure out this patch business. Is there anything I can do to help test it? I'll be happy to test the module once it's patched and complete for downloading for us non-techie types. :)

Thanks,
Walt

bomarmonk’s picture

I am having trouble with the "legal" block or link that I have enabled, and the links to legal module settings. It might be a base path issue, as this has affected many modules and themes.

Basically, the path to my dislcaimer is okay on the homepage, but on subordinate paths, the link actually changes to reflect the current path and becomes invalid. I am also having trouble with the settings for this module. I can't seem to access them with the update to 4.7.

Also, if I'm not mistaken, the SQL should be updated to UTF8, instead of Latin.

Thanks for any further patches or assistance.

jorditr’s picture

So, how far is that last jun-9 cvs release from 4.7?

bomarmonk’s picture

June 9th? I only see commits from November 2005, even though the date on CVS says April 2006. I think this patch is the latest development and the closest to 4.7 that this module has reached. If I've missed something, please let me know!

Walt Esquivel’s picture

Two questions:
1. From post #11 above by rbrooks00 in which he wrote, New patch is attached for testing.

Has anyone been able to integrate the patch rbrooks00 submitted into a module that, even if not the official release, could be tested by non-patch-literate folks like me? ;) I'd like to test the module and tried learning how to patch but my brain cells did not cooperate! ;)

2. From post #14 above by bomarmonk in which he wrote, Also, if I'm not mistaken, the SQL should be updated to UTF8, instead of Latin.

Does the SQL need updating to UTF8?

Thank you,
Walt

rbrooks00’s picture

Not sure if anyone has tested the patch, but I do know that Robert is working on making this officially available for 4.7. I think it'll be ready in the next few days.

robert castelo’s picture

Yes, I'll be releasing the 4.7 update to Legal module in the next day or two, and will be looking to get as much feedback as possible when it does come out - stay tuned.

robert castelo’s picture

Assigned: jaxxed » robert castelo
Status: Active » Fixed

OK, I've uploaded a major rewrite of the module to cvs which is for Drupal 4.7.

Remember that cvs is for development code, so don't use this on a live site unless you've tested it thoroughly on a dummy site.

I'll wait a week for feedback on the cvs version, and if all goes well do a full 4.7 release in a week.

Anonymous’s picture

Status: Fixed » Closed (fixed)
Walt Esquivel’s picture

Version: » master

Hi Robert,

First off, thank you for your fine work on the legal module. I have one more question.

You wrote:

Remember that cvs is for development code, so don't use this on a live site unless you've tested it thoroughly on a dummy site.

Well, I looked for the "full 4.7 release" per your comment above but only found the CVS version which was last updated 6/23/06. Is the CVS version OK to use or should I wait for your full 4.7 release?

Thanks again!
Walt

Walt Esquivel’s picture

Status: Closed (fixed) » Active

Hmmm... I don't see the 4.7 version under the Legal versions listed at http://drupal.org/node/3508/release

Are we going with a CVS version as acceptable for 4.7 purposes? If so, great! If not, I'll wait patiently. : )

Thank you,
Walt

robert castelo’s picture

Well, I haven't got any feedback on the cvs version (4.7), either positive or negative. So I presume it's in use and no bugs have been found, although there's no way to tell how many times it's been downloaded and installed.

I'll create a 4.7 release today, and if I have time I may even write some decent documentation (README.txt).

robert castelo’s picture

Status: Active » Closed (fixed)
mwu’s picture

Thank you for updating it. I just downloaded the 7-16 version and I can confirm it appears to work.
1) I could make it and it shows up at legal
2) The email is sent (and received) if the box is checked.
3) A message is displayed if the box is not checked.

mwu’s picture

I just confirmed
4) the additional checkbox (I only tried 1) works