Suri (Strict URI) is an attempt to reduce "duplicate content" on websites.

The concern

Let's suppose you wrote you code and defined a menu 'mymodule/%' which has as occurence 'mymodule/example'.
The way drupal works make 'mymodule/example' and 'mymodule/example/test' to be treated by the same 'page callback'.
In some case, 'mymodule/example/test' will be treated by Google as duplicate content.

There's another case where you menu doesn't deal with parameters like 'page' or 'utm' or another unwanted. For example, on most urls, we can add an extra parameters (look drupal.org/node/21951 and drupal.org/node/21951?page=12). The extra parameters in the latter case is 'page', which is not performed by the page. Thus making it unecessary.

Both example could lead to duplicate content if indexed by google.

How does suri try to resolve it ?

Each drupal menu has a fixed number parts. When a url is given and if the max parts of the path is more than what is defined in the menu, then Suri try to guess the nearer parent and redirect the user to it.
If a menu is configured to accept only specific arguments, anytime the menu will be called with unwanted argument, the user will be redirect to the nearer parent of the menu.

How can you specify which extra arguments you deal with ?

Extra arguments can be defined when building a menu, through the hook_menu.
A new key Suri arguments can be the same way page arguments is given.
If you're not an administrator, you can specify arguments for every menu drupal through 'admin/settings/suri/urls'.

How can you test it ?

A simple test can be done as follow:
Take any node you have and hit this 'node/xxx/testus' (where xxx is your node id).

Next

A more finished description is to come.

Here is the sandbox: http://drupal.org/sandbox/wtouomi/1436698

CommentFileSizeAuthor
#10 drupalcs-result.txt475 bytesnmudgal

Comments

klausi’s picture

The response time for a review is now approaching 4 weeks. Get a review bonus and we will come back to your application sooner.

wilmar81’s picture

Yes, I read the note about review bonus. And i'm reading everything to make a good review.

Thank you.

eugene.ilyin’s picture

Status: Needs review » Needs work

Hello. I spent manual review of your module.

  • Implements hook_install: use dot after end of comment. In other places also. Please read document about hook comments http://drupal.org/node/1095546.
  • // do not work on ajax requests: Inline comment must start from capital character.

I could only find these defects.

wilmar81’s picture

Thank you Eugene. I fixed your remarks and added one more comment.

wilmar81’s picture

Status: Needs work » Needs review

Oups! I forgot to change its status. Done now.

wilmar81’s picture

Switched from branch master to 6.x-1.x
see http://drupal.org/node/1436698/git-instructions/6.x-1.x for cloning the project

shivachevva’s picture

Status: Needs review » Needs work

* README.txt is missing, see the guidelines for in-project documentation.
* Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

FILE: ...l-7-pareview/sites/all/modules/pareview_temp/test_candidate/suri.module
--------------------------------------------------------------------------------
FOUND 10 ERROR(S) AND 11 WARNING(S) AFFECTING 21 LINE(S)
--------------------------------------------------------------------------------
66 | WARNING | A comma should follow the last multiline array item. Found:
| | 100
77 | WARNING | A comma should follow the last multiline array item. Found:
| | MENU_LOCAL_TASK
85 | ERROR | Whitespace found at end of line
117 | ERROR | Whitespace found at end of line
123 | ERROR | Inline doc block comments are not allowed; use "// Comment"
| | instead
130 | ERROR | Inline doc block comments are not allowed; use "// Comment"
| | instead
144 | ERROR | Inline doc block comments are not allowed; use "// Comment"
| | instead
154 | ERROR | Inline doc block comments are not allowed; use "// Comment"
| | instead
176 | WARNING | A comma should follow the last multiline array item. Found: )
182 | WARNING | A comma should follow the last multiline array item. Found: )
187 | WARNING | A comma should follow the last multiline array item. Found: )
193 | ERROR | Missing function doc comment
224 | WARNING | A comma should follow the last multiline array item. Found: )
244 | ERROR | Whitespace found at end of line
245 | ERROR | Last parameter comment requires a blank newline after it
248 | ERROR | Whitespace found at end of line
263 | WARNING | A comma should follow the last multiline array item. Found: )
268 | WARNING | A comma should follow the last multiline array item. Found: )
292 | WARNING | A comma should follow the last multiline array item. Found:
| | 'force'
318 | WARNING | A comma should follow the last multiline array item. Found: )
323 | WARNING | A comma should follow the last multiline array item. Found: )

wilmar81’s picture

Status: Needs work » Needs review

Fixed all pareview warnings.

wilmar81’s picture

I added pagination on admin/settings/suri/urls, to help navigate through available paths listing.

nmudgal’s picture

Status: Needs work » Needs review
StatusFileSize
new475 bytes

Attached are the result of automated reviews.
Manual Review:

  • suri_pagination(): You should use theme_pager for pagination rather than creating pager on your own.
  • Line no 7: Text should say "duplicate" rather than "druplicate"
  • suri_install(): no need to set variables, you can use default values with variable_get() anyway.
nmudgal’s picture

Status: Needs review » Needs work
wilmar81’s picture

Thanks for your review nmudgal.

I have a small comment on this. As you may have noticed, theme_pager is used in conjunction with pager_query. I know how I can modify some of its variables to use it the way I want. But I thought I would be a better solution to build my own pagination system, since my items are not retrieved through pager_query or one of its likes.

Nevertheless, I removed variable_set in suri.install and fixed errors in suri.css.

prashantgoel’s picture

Status: Needs review » Needs work

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.


FILE: ...upal-7-pareview/sites/all/modules/pareview_temp/test_candidate/suri.css
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  7 | ERROR | Multiple CSS properties should be listed in alphabetical order
 10 | ERROR | Multiple CSS properties should be listed in alphabetical order
 11 | ERROR | Multiple CSS properties should be listed in alphabetical order
 18 | ERROR | Multiple CSS properties should be listed in alphabetical order
 20 | ERROR | Multiple CSS properties should be listed in alphabetical order
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

patrickd’s picture

Status: Needs work » Needs review

don't block deeper reviews because of these few issues found by automated tools

novalnet’s picture

Hello,

Manual review :

1. Please mention your git clone url in project page.
Git clone url : git clone --branch 6.x-1.x http://git.drupal.org/sandbox/wtouomi/1436698.git
2. In suri.module , function suri_pagination() returns "return '';" =>seems this is not in standard. Please use boolean as return values .
3. Please Use t() for all texts used in suri.module.
4. Also avoid using HTML inside t().

patrickd’s picture

Status: Needs review » Closed (duplicate)

It appears that there have been multiple project applications opened under your username:

https://drupal.org/node/1254718

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary (younger) application has been marked as 'closed(duplicate)', with only one application left open.

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

avpaderno’s picture

Title: Suri » [D6] Suri
Related issues: +#1254718: [D6] Lognode