Views hst filter

Hi,

I ran your code through this automated tool to check against coding standard, number of spaces in tab, syntax, newline at end of file etc

http://ventral.org/pareview/httpgitdrupalorgsandboxserhouse1832320git

There are a lot of errors and warnings but they mostly look simple enough, such as class names name needs to start with a capital letter, comments longer than 80 characters etc. Have a go at speeding through the issues list on the automated review. I have done it for one of my modules and found it was a lot easier and quicker todo that I had expected.

Hii,

Did you fix the code according to the suggestions of the automated code review yet?
http://ventral.org/pareview/httpgitdrupalorgsandboxserhouse1832320git

Some points noted there are really important. Also indentation issues are quite necessary to be fixed as this code will be used to collaborate with other contributors some other times and hence its very important to have a common standard.

Here are some points that should be taken care of:

1.Remove the translations folder, translations are done on http://localize.drupal.org
2.Remove "version" from the info file, it will be added by drupal.org packaging automatically.

Highlights of the indentation and styling issues that needs to be taken care of:

1. Line should not exceed 80 characters
2. Files must end in a single new line character
3. Function comment short description must be on a single line
4. No whitespace at the end of a line

I would suggest you to go through the output of the automated code review and fix as many errors as you can.

Thanks,
Aritra

If you can work through all the errors and warnings you can then let us know when the repo is up to date I would be happy to look at the code again and see what I can help out with.

The code is looking better but the automated review is still bringing up a few issues that can be fixed quickly. All the issues in views_hst_filter.module are the same spacing issue with arrays for example.

With views_hst_filter_handler_filter.inc a lot of lines seem to generate 2 errors/warnings as only 27 lines are affected with issues.

You will get more people interested in doing manual reviews once the automated review is coming up clean. Otherwise a manual review would just pick up all the same issues.

It seems that you have the module from kervi. I already uploaded the code. You can see a cleaner version of this module here.

You have to work on "your" code so don't mark it as "needs review".

It appears you are working in the "vental_org" 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.
There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the vental_org branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some issues with your code (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. You have to get a review bonus to get a review from me.

Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards

sites/all/modules/pareview_temp/./test_candidate/views_hst_filter_handler_filter.inc:
 +168: [minor] use lowercase html tags to comply with XHTML

Status Messages:
 Coder found 3 projects, 3 files, 1 minor warnings, 0 warnings were flagged to be ignored

FILE: ...odules/pareview_temp/test_candidate/views_hst_filter_handler_filter.inc
--------------------------------------------------------------------------------
FOUND 34 ERROR(S) AFFECTING 21 LINE(S)
--------------------------------------------------------------------------------
  12 | ERROR | Class name must begin with a capital letter
  12 | ERROR | Class name must use UpperCamel naming without underscores
  15 | ERROR | Class property $is_alone should use lowerCamel naming without
     |       | underscores
  20 | ERROR | No scope modifier specified for function "init"
  34 | ERROR | Method name "views_hst_filter_handler_filter::admin_summary" is
     |       | not in lowerCamel format, it must not contain underscores
  34 | ERROR | No scope modifier specified for function "admin_summary"
  48 | ERROR | Method name "views_hst_filter_handler_filter::option_definition"
     |       | is not in lowerCamel format, it must not contain underscores
  48 | ERROR | No scope modifier specified for function "option_definition"
  61 | ERROR | Function comment short description must be on a single line
  64 | ERROR | Method name "views_hst_filter_handler_filter::options_form" is
     |       | not in lowerCamel format, it must not contain underscores
  64 | ERROR | No scope modifier specified for function "options_form"
  84 | ERROR | Method name "views_hst_filter_handler_filter::expose_form" is
     |       | not in lowerCamel format, it must not contain underscores
  84 | ERROR | No scope modifier specified for function "expose_form"
 120 | ERROR | Method name "views_hst_filter_handler_filter::expose_validate"
     |       | is not in lowerCamel format, it must not contain underscores
 120 | ERROR | No scope modifier specified for function "expose_validate"
 126 | ERROR | Method name "views_hst_filter_handler_filter::expose_submit" is
     |       | not in lowerCamel format, it must not contain underscores
 126 | ERROR | No scope modifier specified for function "expose_submit"
 134 | ERROR | Method name "views_hst_filter_handler_filter::exposed_info" is
     |       | not in lowerCamel format, it must not contain underscores
 134 | ERROR | No scope modifier specified for function "exposed_info"
 140 | ERROR | Method name "views_hst_filter_handler_filter::exposed_form" is
     |       | not in lowerCamel format, it must not contain underscores
 140 | ERROR | No scope modifier specified for function "exposed_form"
 233 | ERROR | Function comment short description must be on a single line
 238 | ERROR | Method name "views_hst_filter_handler_filter::exposed_validate"
     |       | is not in lowerCamel format, it must not contain underscores
 238 | ERROR | No scope modifier specified for function "exposed_validate"
 290 | ERROR | No scope modifier specified for function "query"
 324 | ERROR | Missing parameter type at position 1
 331 | ERROR | Method name "views_hst_filter_handler_filter::is_valid_tid" is
     |       | not in lowerCamel format, it must not contain underscores
 331 | ERROR | No scope modifier specified for function "is_valid_tid"
 347 | ERROR | Method name "views_hst_filter_handler_filter::get_term_child" is
     |       | not in lowerCamel format, it must not contain underscores
 347 | ERROR | No scope modifier specified for function "get_term_child"
 363 | ERROR | Function comment short description must be on a single line
 367 | ERROR | Missing parameter type at position 1
 370 | ERROR | Method name "views_hst_filter_handler_filter::check_for_alone"
     |       | is not in lowerCamel format, it must not contain underscores
 370 | ERROR | No scope modifier specified for function "check_for_alone"
--------------------------------------------------------------------------------

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

I would advise you to delete your sandbox and wait for kervi's module because he is the original author of this module.

Let's close this issue because this is not your code and this is not the sense behind project applications.

But kervi is the author from the module, take a look here.

yes, you are the original author.

but you give me all the rights about this module, remember?

but, no problem. i don't have time to mantain it, so you are the boss.

Interesting, sorry ser_house, didn't know that... It's heavy from kervi do take money for code which isn't written by him. Kervi then you should delete your sandbox right?

Really sorry for all this story. Never again.

Sandbox deleted.

Kervi should also stop selling it! Crazy story...

And ser_house should get a review bonus so that this module can get faster reviewed.

Cross posting relevant HS issue: http://drupal.org/node/1170192

So basically the CodeCanyon version would have thrown up these errors listed above?

It's sad that Kervi made no mention of purchasing/commissioning this himself and set up a sandbox effectively to stop others working on a solution... all the while selling somebody else's work on CC. I appreciate entrepreneurship, but basically it's held up the community.

no, not exactly.

1. i have bough the module from ser_house.
2. he give me the right to do with it all that i want.
3. with 2 new features, i have published the module on codecanyon and started to sell.
4. i have added another feature.
5. yannickoo have bough my version of module, and have created a sandbox on d.org. GPL... :(
6. i have reclaimed the authorship, and i have created a new sandbox,for at least, mantain the module.
7. the original author of module have appeared. with new sandbox.
8. i have removed my sandbox to not interferr.
9. the original author have some problems

yannickoo, thank you for your message. I am sorry, but I will not more nothing to do. I will likely simple remove project through some time, than I will be make class name and functions in lowerCamel format.

10. for now, both sandboxes are removed. If somebody will, he can start again (not me). then new sandbox will be promoved to full project, i'll remove the module from codecanyon.

That is right. The views maintainer dawehner told me that this is absolutely okay and they cannot change the views api because of the coding standards.

Hey guys, this feature is really needed and if it has to exist outside of the hierarchical select module at least for now, fine.

The Drupal way is collaboration and I;m not seeing much here.

What would ideally happen:

- We start working collaboratively in this sandbox: http://drupal.org/sandbox/ser_house/1832320
- kervi adds an issue and a patch with the new features his version of the module has
- yannickoo adds an issue and a patch to clean up the code
- seb_house commits those patches and gives git attribution to kervi and yanickoo (easy to do with dreditor)
- the module gets promoted to a full module or a patch id done for hierarchical select
- everyone has attribution on thier d.o profile for their work on the modules
- possibly with time co-maintainers?

Let's make it happen!

+1 for @ipwa! :)

new features:
1. counter of related nodes for each term
2. exclude empty term
3. unique id for each widget.

So here is the patch for the code clean up. The maintainer should remove the master branch and create a 7.x-1.x branch instead. The README.txt file should be written in english and not in russian.

As ipwa mentioned in #27 the git commit message should be git commit -m "Issue #1841548 by yannickoo: Clean up code." --author="yan_nick <yan_nick@531118.no-reply.drupal.org>".

@ser_house: Please don't take this personally. I think your code is good and most importantly it does work. I know the review process can be daunting but we are just trying to do our best so your application is successfull. If you don't address the issues about the Git branch (you need to add a 7.x-1.x branch), and the issues mentioned in the automated review for views_hst_filter_handler_filter.inc from #9, the Git maintainers won't approve your project. If it was up to me you would get your application approved and we could start working on this issues on the live project through the issue queue, but unfortunately that's not how it works, all those little issues have to be solved before the application can be successfull. If you want to discuss more about this I'm happy to do an irc chat or an email conversation if you use my contact form, it's not that the Git maintainers don't want you to promote the project it's just that since Drupal get's so many project applications the standards for approval are quite high, but I'm sure we'll get there if we do some minor changes. Look at the guidelines to review applications to get an idea: http://drupal.org/node/894256

I will add issues to the sandbox and report back so we can start adding patches.

Great! We are geting somewhere these are the two issues:

#1841548: Clean up code
#1841560: New features from @kervi

The @yannickoo issue already has a aptch so we are just waiting for @kervi to add his patch. Once those two issues get resolved we can mark this as 'reviewed & tested by the community" so the Git maintainers can review it.

Thanks to everyone specially @seb_house for starting this useful sub-module and to @kervi and @yanickoo for helping out.

I hope this code will be removed from codecanyon if not now at least when it's approved and goes on to be a full project on d.o

Awesome.

The Git issue was addressed and the code clean up, so we are nearly there.

You are right those features don't necessarily have to be in, in order for this to be a full project (although it might help when the git admins do their review)

I will do a full review tomorrow and if I find everything ok, I'll mark it as 'reviewed and tested'.

There are even multiple code issues. See pareview. You can ignore the classes stuff but the indentation, function descriptions and the max. 80 characters are important.

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

• Run coder to check your style, some issues were found (please check the Drupal coding standards). See attachment.
• Drupal Code Sniffer has found some issues with your code (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. You have to get a review bonus to get a review from me.

Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards

sites/all/modules/pareview_temp/./test_candidate/views_hst_filter_handler_filter.inc:
 +124: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
 +217: [minor] use lowercase html tags to comply with XHTML

Status Messages:
 Coder found 3 projects, 3 files, 1 critical warnings, 1 minor warnings, 0 warnings were flagged to be ignored

FILE: ...al-7-pareview/sites/all/modules/pareview_temp/test_candidate/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 20 | WARNING | Line exceeds 80 characters; contains 87 characters
--------------------------------------------------------------------------------


FILE: .../sites/all/modules/pareview_temp/test_candidate/views_hst_filter.module
--------------------------------------------------------------------------------
FOUND 10 ERROR(S) AFFECTING 10 LINE(S)
--------------------------------------------------------------------------------
 67 | ERROR | Array indentation error, expected 4 spaces but found 0
 68 | ERROR | Array indentation error, expected 4 spaces but found 0
 69 | ERROR | Array indentation error, expected 4 spaces but found 0
 70 | ERROR | Array indentation error, expected 4 spaces but found 0
 71 | ERROR | Array indentation error, expected 4 spaces but found 0
 72 | ERROR | Array indentation error, expected 4 spaces but found 0
 73 | ERROR | Array indentation error, expected 4 spaces but found 0
 74 | ERROR | Array indentation error, expected 4 spaces but found 0
 75 | ERROR | Array indentation error, expected 4 spaces but found 0
 76 | ERROR | Array indentation error, expected 4 spaces but found 0
--------------------------------------------------------------------------------


FILE: ...odules/pareview_temp/test_candidate/views_hst_filter_handler_filter.inc
--------------------------------------------------------------------------------
FOUND 81 ERROR(S) AND 2 WARNING(S) AFFECTING 54 LINE(S)
--------------------------------------------------------------------------------
  11 | ERROR   | Class name must begin with a capital letter
  11 | ERROR   | Class name must use UpperCamel naming without underscores
  14 | ERROR   | Class property $is_alone should use lowerCamel naming without
     |         | underscores
  19 | ERROR   | No scope modifier specified for function "init"
  33 | ERROR   | Method name "views_hst_filter_handler_filter::admin_summary"
     |         | is not in lowerCamel format, it must not contain underscores
  33 | ERROR   | No scope modifier specified for function "admin_summary"
  47 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::option_definition" is not in
     |         | lowerCamel format, it must not contain underscores
  47 | ERROR   | No scope modifier specified for function "option_definition"
  65 | ERROR   | Function comment short description must be on a single line
  68 | ERROR   | Method name "views_hst_filter_handler_filter::options_form" is
     |         | not in lowerCamel format, it must not contain underscores
  68 | ERROR   | No scope modifier specified for function "options_form"
 112 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::has_extra_options" is not in
     |         | lowerCamel format, it must not contain underscores
 112 | ERROR   | No scope modifier specified for function "has_extra_options"
 120 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::extra_options_form" is not
     |         | in lowerCamel format, it must not contain underscores
 120 | ERROR   | No scope modifier specified for function "extra_options_form"
 156 | ERROR   | Method name "views_hst_filter_handler_filter::expose_validate"
     |         | is not in lowerCamel format, it must not contain underscores
 156 | ERROR   | No scope modifier specified for function "expose_validate"
 156 | ERROR   | There should be no white space after an opening "{"
 156 | ERROR   | There should be no white space before a closing "}"
 162 | ERROR   | Method name "views_hst_filter_handler_filter::expose_submit"
     |         | is not in lowerCamel format, it must not contain underscores
 162 | ERROR   | No scope modifier specified for function "expose_submit"
 170 | ERROR   | Method name "views_hst_filter_handler_filter::exposed_info" is
     |         | not in lowerCamel format, it must not contain underscores
 170 | ERROR   | No scope modifier specified for function "exposed_info"
 176 | ERROR   | Method name "views_hst_filter_handler_filter::exposed_form" is
     |         | not in lowerCamel format, it must not contain underscores
 176 | ERROR   | No scope modifier specified for function "exposed_form"
 219 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"
 254 | WARNING | Line exceeds 80 characters; contains 87 characters
 271 | WARNING | Line exceeds 80 characters; contains 83 characters
 284 | ERROR   | Function comment short description must be on a single line
 289 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::exposed_validate" is not in
     |         | lowerCamel format, it must not contain underscores
 289 | ERROR   | No scope modifier specified for function "exposed_validate"
 342 | ERROR   | No scope modifier specified for function "query"
 380 | ERROR   | Method name "views_hst_filter_handler_filter::is_valid_tid" is
     |         | not in lowerCamel format, it must not contain underscores
 380 | ERROR   | No scope modifier specified for function "is_valid_tid"
 394 | ERROR   | Method name "views_hst_filter_handler_filter::get_term_child"
     |         | is not in lowerCamel format, it must not contain underscores
 394 | ERROR   | No scope modifier specified for function "get_term_child"
 417 | ERROR   | Data type of return value is missing
 419 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 420 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::get_vocabulary_levels" is
     |         | not in lowerCamel format, it must not contain underscores
 420 | ERROR   | No scope modifier specified for function
     |         | "get_vocabulary_levels"
 435 | ERROR   | Last parameter comment requires a blank newline after it
 440 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::is_single_filter" is not in
     |         | lowerCamel format, it must not contain underscores
 440 | ERROR   | No scope modifier specified for function "is_single_filter"
 455 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 455 | ERROR   | Function comment short description must be on a single line
 455 | ERROR   | Function comment short description must end with a full stop
 456 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 457 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 458 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 458 | ERROR   | Doc comment for var Term does not match actual variable name
     |         | $tid at position 1
 458 | ERROR   | Parameter comment must be on the next line at position 1
 459 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 460 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 460 | ERROR   | Doc comment for var TRUE does not match actual variable name
     |         | $child_count at position 2
 460 | ERROR   | Parameter comment must be on the next line at position 2
 461 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 462 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 463 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 464 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::get_term_nodes_count" is not
     |         | in lowerCamel format, it must not contain underscores
 464 | ERROR   | No scope modifier specified for function
     |         | "get_term_nodes_count"
 467 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"
 480 | ERROR   | No space before comment text; expected "//
     |         | $query->condition('n.type', 'node_type', '=');" but found
     |         | "//$query->condition('n.type', 'node_type', '=');"
 489 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 490 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 491 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 491 | ERROR   | Last parameter comment requires a blank newline after it
 491 | ERROR   | Missing parameter type at position 1
 492 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 493 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 494 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 495 | ERROR   | Expected 3 space(s) before asterisk; 2 found
 496 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::term_get_children_ids" is
     |         | not in lowerCamel format, it must not contain underscores
 496 | ERROR   | No scope modifier specified for function
     |         | "term_get_children_ids"
 500 | ERROR   | Expected "if (...) {\n"; found "if (...){\n"
 501 | ERROR   | Expected "foreach (...) {\n"; found "foreach(...) {\n"
 513 | ERROR   | Last parameter comment requires a blank newline after it
 518 | ERROR   | Method name
     |         | "views_hst_filter_handler_filter::build_options_list" is not
     |         | in lowerCamel format, it must not contain underscores
 518 | ERROR   | No scope modifier specified for function "build_options_list"
 533 | ERROR   | Expected "if (...) {\n"; found "if(...){\n"
 539 | ERROR   | Expected "if (...) {\n"; found "if(...){\n"
 556 | ERROR   | Method name "views_hst_filter_handler_filter::build_widget_id"
     |         | is not in lowerCamel format, it must not contain underscores
 556 | ERROR   | No scope modifier specified for function "build_widget_id"
 558 | ERROR   | String concat is not required here; use a single string
     |         | instead
--------------------------------------------------------------------------------

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

This is sad news. So many people are waiting for this functionality in D7.
Are there any decent alternatives out there?

@ChristopheDG: I don't understand, ser_house has a sandbox project called Views hst filter:
http://drupal.org/sandbox/ser_house/1832320
It's a very useful module, and can be used on D7. But it's still a sandbox project yet, but it shouldn't scare you.

Git maintainers, this application has been reviewed and tested by the community, please grant full Git access to @ser_house.

ser_house: you marked this application as "won't fix". Does this mean you do not want this project be promoted to a full project and do not want to get full git access?
If you still want to continue with your application, please get some review bonus by reviewing other applications.

Btw., the project application can last a long time. You create this issue about 9 weeks ago, which is not very long. Just have a look at all the other applications waiting for input. We are quite busy at the moment (which is unfortunately a permanent state) so keep patient and don't give up ...

What is holding this one up?

Let's get this into Hierarchical Select proper. See #1170192-239: Port Hierarchical Select Taxonomy Views (for exposed filters in Views) to Drupal 7.

@ser_house :
:( why won't your module be available here? I think your module is great, and should be continued to be discussed and developed here, it would even be better if it got to a full release... :)

@ser_house :
But what made you angry with drupal.org? :( This way we can not easily participate in creating patches, helping your efforts, etc. :(
And I think many users don't speak Russian (e.g. unfortunately I don't).
I would be really sad if you removed your project.

ser_house, what is the problem now? Why you don't want to promote your sandbox as full project? I don't understand that.

BTW thank you for mention us on your page ;)

I also can put this on drupal.org, what do you think?

Okay, that is our opinion. All other users can use the new Simple hierarchical select.

@ser_house :

Should I to dance for this?

So, I have deleted the module.

I'm surprised at your negative approach. I read the comment you got offended at, where #1410826: [META] Review bonus was linked and where stBorchert said they were really busy currently "so keep patient and don't give up"... this comment didn't contain any offensive stuffs, and just stated that it can take a long time for your module to get reviewed and for your module to be promoted to a full release, because the queue is really huge, but you can speed up this process by getting some review bonus. Noone said that your project will be deleted if you don't review other modules.

I don't understand why you didn't simply keep this module here in your own sandbox if you don't have time to deal with other modules' reviews (that's absolutely understandable). This way you helped other people and improved your reputation! But now you deleted this useful module... :((

I also can put this on drupal.org, what do you think?

@yannickoo: the Simple hierarchical select was developed from scratch or forked from ser_house's module?

the Simple hierarchical select was developed from scratch or forked from ser_house's module

The module has been built from scratch and takes a different approach (no dependency to Hierarchical select, for taxonomy only).

@stBorchert : thanks for your efforts, I will check out your module, I hope this will be a good alternative. :))

@stBorchert: Ok, thanks, looks greate

.

@pinkonomy: you need to add Simple hierarchical select filter to views filters. This is a new kind of filter and you can find it by simply typing hierarchical in Search field while adding filter. Then you'll get all the settings you need hopefully.

great alternative: http://drupal.org/project/shs

Enough.

shs works for nodes, not users. its not a complete solution for the hs view exposed filters issue