Taxonomy Views Switcher

would need it..

Looking forward to this!

simg, why did you change the status?

Hi simg!

I'm not a pro at this but I can give you a quick review based on my knowledge this far :-).

First of all, I like the idea and I think I absolutely could make use out of this someday.

LICENSE.txt
Remove this file, it's added automatically by the Drupal packager.

$views_weight = db_select('system') in .install
I think you should use a static select query here and not a dynamic select query. From what I understand, you should only use dynamic select queries when you want other modules to be able to hook into a specific query and make changes to it, witch is not necessary here from what I can see (I may be wrong). See http://drupal.org/node/310072 for more information. I'm not sure about the db_update() though, think it's okay!

Drupal Coding Standards
Please run your module through the Coder module and correct what's asked by it. It checks your code so it follows all of Drupal's coding standards etc.

Function documentation (I guess the Coder module will point this out?)
Missing doc blocks and comments for functions, please explain a little bit more what every function is doing.

// It has a path or it is not eligable.
Typo :). Run the module though a spell checker to see if there is any more of'em.

elseif (substr($path,0,4) == 'vid/') {
Use the Drupal versions of functions like this, see for example http://api.lullabot.com/drupal_substr/7 .

Overall, the code looks really good, great job!

As I said, this is only a fast review of the code and I guess there is more things for more experienced reviewers to add, but I hope this can get you started :).

BTW, Thank you for sharing this modules with the community. I like the module and I think you'll make a great job with it in the future.

/m

I'm really glad someone reviewed. I've been looking forward to this for some time now. Thanks again to simg!

No activity in several months. Reopen and set the status to "needs review" if you are still pursuing this application.

@nylin, the drupal_substr() docs mention that "for cutting off a string at a known character/substring location, the usage of PHP's normal strpos/substr is safe and much faster." This seems like an instance where substr() would be appropriate.

PS, @simg: Any progress? I'd be willing to take a look at those coding standards / documentation issues if that's all that's holding you back.

Here's a couple of patches to change the db_select to a db_query in the .install file, and modify some of the comments in the main .module file for accuracy / spelling (mostly as pointed out by @nylin above).

(I guess maybe that should go in an issue on your sandbox project?)

You need to set the status to "needs review" if you want to get a review.

Get a review bonus and I'll review your project right away.

There is already a similar module that works very well:

http://drupal.org/project/taxonomy_display

What are the differences with this one?

Thank you very much

Hi,

You are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. . This blog also gives really useful tips about creating project application page.

Would be great in case you can first create the version specific branch so that I can review the complete code.

Hi,

Please see the automated report generated by: http://ventral.org/pareview/httpgitdrupalorgsandboxsimg1106944git. Please make the changes as well as the changes mentioned in the above comment, so that we can review the correct branch.

First of all, I like the minimalist nature of this module. I think the less intrusive approach justifies its existence, even though there are similar modules out there.

I've installed, tested, and read through the code. There is only one suggestion that I'd make, and that is to correctly set the access permissions. I think they should be changed from "anyone can view" to "access content" as the default taxonomy view doesn't apply this either. I imagine that would look something like this:

  $items['taxonomy/term/%'] = array(
    'page callback' => 'taxonomy_views_switcher_render_view',
    'page arguments' => array(2),
    'access callback'  => 'user_access',
    'access arguments' => 'access content',
  );

This would allow the removal of the taxonomy_views_switcher_view_access() function.

Also, one very minor point. Could you remove the executable flag from the files? It doesn't really effect anything much, but it occasionally causes problems if you're using Git with Windows (like I sometimes do).

I'd love to see this module published, as it would come in handy for at least a couple of sites I'm working on.

Hi,

This is a very nice module and is very helpful, have not found any fault, however as it is like "taxonomy_views_integration" module, it would have been great if you can contribute a patch to that module and maintain that part actively, in such case we will get one module doing everything. This is my personal though though.

Have not found any coding error with automatic testing, experts can advice on manual testing.

All the best and Nice work.

Changing to RTBC as it works as advertised for me. If the lack of permissions checking is justified, then I can't see any other issues with this module. I'd like to see it promoted so I can start using it.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Automatic review

Ok

Manual review

no security issue as far as I see

Minnor issues

module file - line 9. Commenting a hook function is done as follows:
Implements hook_menu().
In any case that function is not a hook_menu() implementation.
Please check all other comments for easier documentation.

Major issues

As mentioned at #25, there should be an access parameter on taxonomy_views_switcher_menu_alter(&$items).
Otherwise intranet websites will have their content displayed even if anonymous users don't have the permission to access it.

Other questions

The module you made reference is named "Taxonomy Views Integrator" (README.txt). Beside that, those two modules does basically the same thing but "Taxonomy Views Integrator" comes with an administration interface.

Personally I think that you made a great job in here and that the taxonomy personalization on Drupal slows down the production and your module can be handy but... one of the things promoted at Drupal is collaboration instead of competition. That approach tend to provide high quality modules instead of having lots of them doing the same thing.
I strongly advise you to get in contact with TVI maintainers to add or create a fork of that module.

You can read more here:
http://drupal.org/node/894256

This is a group created to solve the duplication problems (huge lists):
http://groups.drupal.org/similar-module-review

If you really want to get this module you could provide different assets that doesn't exist already ( as said on this post, similarity is not duplication ) and explicitly explain the unique benefits of using your module. I think that a module that does the same but is simpler is basically a duplication.

I set this as "needs work" as I understand the access problem and the duplication as blockers.

- Just make a drupal intranet (set 0 permissions to the anonymous users) then go to the url where the taxonomy page should(nt) be displayed without beeing logued. This is a problem in fact.

- Well the functionality is the same... even if its done by another way... similarity/difference is based on the result (functionnality) not on the way to achieve it.
What would be the point of having a 100 modules that does all the same result but using each time a completely different code behind ?

Well its just my personal opinion as reviewer. Concerning duplicate functionalities there is a group created specifically for that. They have more experience on evaluating this particular case, maybe you can go there and ask them, their opinion on that point will be more accurate than mine for sure. http://groups.drupal.org/similar-module-review

I just wanted to say I'm using this module and I'll love to see it published.
The title of the views is lost when using the module, and right now I'm adding a new title using a hook_views_pre_render.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.