Port Legal module to drupal7

No, I'm making some extensive changes to Legal module at the moment, so please hold off starting a D7 port till November.

Is HEAD ready to start porting yet?

Any updates on this? We ready to start the D7 port?

We should clear out the issue queue before starting a port. Working on it.

suscribe

Is it november yet? ;-)
Subscribing.

You guys did mean November 2010 right :)

This works in general but there are a few bugs that I believe need to be fixed. There's one particularly nagging bug that I haven't been able to determine the root cause.

function legal_administration_validate($form, &$form_state) and function legal_administration_submit($form, &$form_state)
do not seem to store the user submitted details in $form_state['values'] leading to all sorts of ugliness accessing the user submitted details using $form_state['complete form'] blah. If anyone spots the root cause of the bug please fix and let me know why.

Cheers.

Fix for legal.admin.inc

$version = legal_version($form_state['values']['version_handling'], $form_state['values']['language']);

and changed your db_insert portion so it's slightly easier to read:

db_insert('legal_conditions')
->fields(array(
'tc_id' => NULL,
'version' => $version['version'],
'revision' => $form_state['values']['revision_id'],
'language' => $form_state['values']['language'],
'conditions' => $form_state['complete form']['conditions']['#value'],
'date' => time(),
'extras' => serialize($form_state['complete form']['extras']['#value']),
'changes' => $form_state['complete form']['changes']['#value'],
))
->execute();

Still loads of other bugs to squash though...

Thanks. File em and I'll fix the ones I can.

subscribe!

You may have noticed I've been going through the issue queue, weekend by weekend, fixing bugs and adding features. A few more to go, and then I'll start on a Drupal 7 release.

Although the code will change, the data structure (database) will remain the same, and although patches won't apply, any work porting Legal to Drupal 7 will be useful, as I can look through the code as a reference.

Thanks for everyone's efforts!

Subscribe!

Subscribing

Subscribe

subscribe

Subscribe

subscribe

I've ported Legal module to Drupal 7, but this patch could be slimmed down quite a bit by making a few basic changes to the D6 version of the module:

• The D6 version should follow coding standards. This module is particularly bad about having mixed spaces and tabs for indentation. Tons of lines also contain trailing whitespace. Comments should all be sentence case, starting with a capitol letter and ending with a period. Since I included a lot of these changes, this patch is versbose in places.
• hook_user() should be split into separate functions such as hook_user_insert(), hook_user_login(), etc. Even though these aren't real hooks in D6, they could be called from hook_user() manually. This would reduce the amount of code just being moved around or unindented in this patch.

In any case this patch seems to get Legal fully working in D7. I've also made a sandbox over in http://drupal.org/sandbox/quicksketch/1173602 to keep up with more changes/fixes to the D7 port, since with 6 months since the last commit to Legal module, I'm not very optimistic that this will make its way back into the official module any time soon.

since with 6 months since the last commit to Legal module

Sorry that assumption was made based on the Legal project page, which for some reason lists the last commit at 26 weeks ago. The Git repo has obviously seen some action more recently than that: http://drupalcode.org/project/legal.git

In any case, at least the sandbox will make it easier to reroll.

Subscribing

So, we have to install d6 version and then upgrade even though we are on a fresh d7 website ?

So, we have to install d6 version and then upgrade even though we are on a fresh d7 website ?

No, just install the D7 version.

subscribe!

I may be noob, but where is the D7 version ? I can only see patches here, and there is no d7 version on this project's page (not even in "view all releases" section) ... so where can I find it ?

Great stuff quicksketch :)

Have cleaned up code format so it complies with coding standards, ran it through Coder set to Minor and only get one exception that I don't want to change (and commented why).

Released as 6.x-8.5, which includes a few other changes as well.

quicksketch would you have time to re-roll the patch to make it easier to see the changes for a D7 update?

That's great Robert! Yes I'd be happy to merge the changes and update the Git repo with the latest. Those code-style inconsistencies were driving me bonkers! I'll roll a patch and see how drastic the diff is and post here.

Because I'm both nice and in a hurry, I'm attaching a functional D7 Legal port, based on what's been posted in this thread to date. It's based on 6.x-8.5, with the sandbox patch from http://drupal.org/sandbox/quicksketch/1173602 and fixes all the blocking issues I found with that.

Works for me on D7.4, no guarantee offered. Probably still bugs lurking in there.

Yeah I tried to merge with the existing 6.x branch but it's an absolute disaster. There are dozens and dozens of confusing conflicts and it's nearly impossible to tell what's useful and what's not. We need something stable to work off of, as my sandbox is now useless when it comes to merging with the existing 6.x branch with all the code-cleanup changes.

Quicksketch thanks for trying!

I'm taking a few days off and plan on porting Legal to Drupal 7 as a little holiday project, will look through your patch as part of the upgrade.

subscribe

subscribe.

subscribe.

subscribe.

Subscribe

Not sure if this is the right place for this, but I applied the patch in #20 and had the module running successfully in d7. I believe I tracked down a bug that started with a 'Notice: Undefined property: stdClass::$lc_id' message.

In legal.admin.inc part of the legal_versions_latest_get function is:

// get latest version for each language
  if (empty($language)) {
    $languages = locale_language_list();    
    foreach ($languages as $language_id => $language_name) {
      $result = db_select('legal_conditions', 'lc')
        ->fields('lc')
        ->condition('version', $current_version)
        ->condition('language', $language_id)
        ->orderBy('revision', 'DESC')
        ->range(0, 1)
        ->execute()
        ->fetchAllAssoc('lc_id');   // shouldn't this be tc_id ?
      $row = count($result) ? (object) array_shift($result) : FALSE;
      $conditions[$language_name] = legal_versions_latest_get_data($row);
    }
  } // Get latest version for specific language.
  else {
    $result = db_select('legal_conditions', 'lc')
      ->fields('lc')
      ->condition('language', $language)
      ->groupBy('language')
      ->orderBy('version', 'DESC')
      ->range(0, 1)
      ->execute()
      ->fetchAllAssoc('lc_id'); // shouldn't this be tc_id ?
    $row = count($result) ? (object) array_shift($result) : FALSE;
    $conditions[$language] = legal_versions_latest_get_data($row);
  }

I changed the two instances above to ->fetchAllAssoc('tc_id') as this was the column name in the legal_conditions

Also in function legal_display_change... in legal.module

$result = db_select('legal_conditions', 'lc')
    ->fields('lc', array('changes'))    //changed to ->fields('lc') 
    ->condition(db_or()
        ->condition('version', $last_accepted['version'], '>')
        ->condition(db_and()
          ->condition('version', $last_accepted['version'])
          ->condition('revision', $last_accepted['revision'], '>')
        )
      )
    ->condition('language', $last_accepted['language'])
    ->orderBy('revision', 'ASC')
    ->orderBy('version', 'ASC')
    ->execute()
    ->fetchAllAssoc('lc_id'); // changed to ->fetchAllAssoc('tc_id');

These changes resolved the notice, and all seems well.

Subscribe

@sitekick can you provide a patch?

Picked through Quicksketch's #20 patch by hand and updated the module, currently just a work in progress, as I haven't had time to test, and Simpletest returns many errors.

Ran out of time do any more work on it at the moment, will carry on next weekend.

Drupal 7 version now available!

Lots of bugs squashed this weekend.

I tested manually, and also updated and ran the automated tests, all features working as far as I could see.

Exception is the Views generated pages which report an error and don't display some of the data. Will need to fix that another day.

Thanks to RasputinJones, l33tdawg, quicksketch, exratione, and sitekick for code and encouragement.

YAY Robert Castelo!! Thanks for all your work!

Thank you very much!!!!

Nice work.

Re #42 If I recall correctly, the errors on the view pages went away when I enabled the Locale module. And all data was present.

Re #40 Sorry I missed your post, not sure I could have provided a patch anyway. Need to read up on making patches and had modded the module pretty heavily to meet some specs.