Drupal 7.x Port

This is on the cards.

Hello, is there any update on the Drupal 7 port of this useful module? thx

Great. Look forward to seeing this module.

Really interested in this. Subscribing.

subscribe.

We've got time to work on this during this week. Let me know and we'll post an update for testing this week.

Here's the patch for Related links D6--> D7. Let me know how this works out

Hi Joe,

Thanks for the patch! However, it fails to apply on my end for the .module and .info files. This is both with Eclipse and the CLI.

Visual review:

• Please stick to recommended Drupal practices during porting. These including adhering to established coding standards. The whitespacing - indentation, usage of TABs etc. - needs to be cleaned up in the patch. Please also avoid Windows line endings.
• Please avoid formatting fixes in general until after the portage.
• Please avoid implementing bug-fixes that are non-essential to the portage. For example, one of the regexes appears to have been modified.
• Please terminate all comments with a period.

Please reroll and resubmit at your convenience.

Cheers,
-K

Hi Karthik,

Tested w/ Eclipse and Command Line. Sorry for the prior noob errors and any that might happen in advance ;-) we'll get to the upgrade sooner than later, -J

Hello Joe :)

Thanks for the updated patch. I have taken the liberty of being nit-picky with my review since you are keen on getting things done just right :) This is primarily a visual review with some basic testing to see if forms etc. are working.

• The patch now applies fine except for the .info file. Was it made against a downloaded install?
• The .info file can now add information linking to the module's configuration page (which can be listed on admin/modules). This is missing.
• The .info file can now add information about module tags using the tags[] syntax.
• The drupal_install_schema and drupal_uninstall_schema calls are unnecessary in relatedlinks.install. Installations fail with an error if they are present.
• The configuration page should be moved to admin/config/relatedlinks.
• All links to the block configuration page need to be updated.
• Debugging code in admin.inc needs to be removed.
• Indented code should always only have 2 spaces from the beginning of the statement rather than just the structure. For example,
  

  $output = theme('table', array(
                'header' => $header,
                'rows' => $rows,
                'attributes' => array(
                    'id' => 'relatedlinks_types_form',
                    'style' => 'width: 100%',
                ),
              ));
  

  The above block should be indented relative to $output rather than theme(). This applies everywhere. Please also avoid unnecessary indentation and breaking up simple statements into multiple lines.

• Please think of a suitable alternative for $the_form :)
• Replacing the use of <> with != in SQL statements is not kosher.
• Please remove the vestigial hook_nodeapi().
• Is the object with links in _relatedlinks_filter() a new feature?
• _relatedlinks_update_discovered_links: What's going on here with the if (true) check?
• _relatedlinks_node_candidates: db_rewrite_sql is necessary.
• The PHPDOC for hook_form_alter appears to be incorrect.

Please also run the patched module through coder to ensure that there are no non-formatting issues that need to be taken care of.

I will stop here for now. I will test functionality in the next go-around.

Many thanks for your efforts,
-K

Zen,

A few points:

Function db_rewrite_sql did not exist in Drupal7 so we are using db_select function and we can use hook_query_alter to alter query created by db_select.

- Object in _relatedlinks_filter() isn’t a new feature. $links variable can be array or object. That’s why we need to check it first.

- We were able to apply the patch for all files. We didn’t have any problem with .info file. Patch was tested using Git command line and Eclipse.

- We ran Coder for this module but we fixed formatting issues only in new code. We didn’t fix any formatting bugs in existing code.

@jbachana does this patch give a functioning module? Is it good enough (in your opinion) for a dev release for consideration?

@Zen how do you think we should move this closer to release?

@mgifford, as this is the first time my team and I are starting to help upgrade D6 to D7 modules on d.o, I would feel most comfortable having Zen approve the work first. We've been successful using the module in our own test environment but this is Zen's module and so he really should approve the patch before it gets committed. Thanks, -J

I ran this all through coder and cleaned up the trivial pieces it revealed. I ran the patch against Git & have recomplied it based on Git.

I got these errors:

Fatal error: Cannot use object of type stdClass as array in:

    foreach ($result as $word) {
       $words[] = $word['word'];
    }

Going here produced unexpected results - /relatedlinks/autocomplete

I think this is how you add links manually, but I couldn't see how to do that.

The Similar links block seems to work as expected, but not sure how to generate the other two.

I made slight changes to the README & other files.

I used mgifford's patch for testing and my notes are further below.

@mgifford: Thanks for jumping in as well! FYI, Joe is holding off on fixing D7 (Coder) formatting warnings until after the upgrade patch has gone through in order to simplify reviews.

@jbachana: Please incorporate mgifford's changes in any updates. If you can avoid merging any unrelated formatting changes, that will be ideal.

relatedlinks.info:

• Version, project info should not be in the .info file. IIRC, this is still being added in by the packaging script.
• tags[] are one per line just as with dependencies. Not sure if anything is actually making use of "tags" though.
• config URL is incorrect (as it needs to tally with mgifford's change).

relatedlinks.module

• Parsed links no longer picks up embedded URLs of the form <a href="/foo">Bar</a> correctly.
• With the search module enabled and the database indexed, enabling discovered links leads to a WSOD on the node page; this appears to happen during the node insert / update. Discovered links have not been tested yet.

relatedlinks.js

• random newline at the top of the file

Cheers,
-K

K,

Here is the update but without the changes from mgifford patch.

-J

ahh, disregard the prior patch, please inspect this one instead. Let me know, thanks! -J

Can you incorporate the patch I produced or are you waiting for me to do this? I'm not sure when to get to this but want to know if this is waiting for me or not.

mgifford I think I missed Zen's note to incorporate your changes - would you like to take mine and do the merge for Zen? I'm out of the office this week so I wouldn't be able to get to it until next week anyway, let me know either way. -J

I can wait till next week for you to incorporate the changes. I can then focus on reviewing it so we can get a bit closer to a release. Thanks!

Hi,

I believe that the patch is good enough for a commit and have gone ahead and done so. Thanks for sticking with this :) I have created a -dev release for the time being. Please test and report any issues.

Do either of you have any immediate plans to add new features/fixes to this module? I believe that things like node_access etc. have been currently introduced as a stop-gap solution. I'd prefer to know your status prior to deciding how to proceed with a release.

Regards,
-K

I can probably dedicate some time to helping out. One thing I was thinking about was that if we're going to divvy up some of the work we might just get more specific about what we'll be working on (mgifford and I). Give me a day or two to think about what functionality we might add, I'm sure you two will have ideas as well. Then, we can create a roadmap for the module and divvy up the work :-)

-J

ahh, Zen did you incorporate mgifford's patch that he produced, since this was not done yet while I was on vacation?

I manually added in the URL change that mgifford introduced for the configuration pages. I believe the rest were Coder recommendations and I took care of them separately. If I missed anything, please let me know.

-K

Thanks Zen. I still don't see the release available from the main page. Is it only available through git?

The dev snapshot should be visible now.

Cheers,
-K

I got this error when installing it:

Fatal error: Call to undefined function _relatedlinks_isenabled_nodetype() in /DRUPAL7/sites/all/modules/contrib/relatedlinks/relatedlinks.module on line 280

That is defined in relatedlinks.module: if (_relatedlinks_isenabled_nodetype($node)) {

After that it worked fine. Pulled down nicely with drush.

Thanks mgifford. Fatal error fixed.

-K

Thanks so much for this great module.

Works great, but I am getting an error when I load chosen content types.

Notice: Trying to get property of non-object in _relatedlinks_update_discovered_links() (line 651 of /home/uitv/html/sites/all/modules/relatedlinks/relatedlinks.module).

This only happens on first load of the page, after which everything is fine.

annikaC, what version of Drupal7 are you using? I can't reproduce this error.

7.4 currently, but will be doing the 7.7 update later today...

Edit: 7.7, fixed. (Probably a caching issue.)