This module creates a +/- tolerance setting when filtering a numeric based view field.

So for example, if you have a number field that is exposed, A person can search for "100" and automatically have it search by plus or minus 10%. So it would return all results between "90" and "110".

Sandbox module page: https://www.drupal.org/sandbox/philbo/2591875

Git git clone --branch 7.x-1.x http://git.drupal.org/sandbox/philbo/2591875.git views_numeric_range_filter

Comments

paboden created an issue. See original summary.

minnur’s picture

Hi Phil,

The module looks good to me. I ran the Coder module and this is what I see:

1. views_numeric_range_filter.module

Line 10: indent secondary line of comment one space [comment_comment_indent]
*/

2. views_handler_filter_numeric_range.inc
Line 98: Control statements should have one space between the control keyword and opening parenthesis [style_control_spacing]
switch($operator) {

paboden’s picture

Thanks @minnur. Committed a fix for those items, plus fixed some variable names and field descriptions that I forgot I fixed on my original project.

PA robot’s picture

Issue summary: View changes
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxphilbo2591875git

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

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

dawehner’s picture

In general this module could be in the future included in views itself, please create an issue on Drupal, let's see whether we can figure that out together.

Nitpick: (views_numeric_range_filter.info):

name = Vews Numeric Range Filter module.
description = Vews Numeric Range Filter module.

Didn't knew that the vews module was a thing ..

One question regarding the implementation:

    if ($this->operator == 'range') {
      $this->query->add_where($this->options['group'], $field, array($min, $max), 'BETWEEN');
    }
    # Would this part of the codepath be ever triggered? Isn't $this->operator 'range' always to call out to op_range() in the first place?
    else {
      $this->query->add_where($this->options['group'],
        db_or()
          ->condition($field, $min, '<=')
          ->condition($field, $max, '>=')
      );
    }
paboden’s picture

@dawehner hahaha "vews". Thank you for catching that. Updated that and updated the description too.

In regards to the second question, This range functionality comes from the op_between function from views core. Mine just auto sets the min and max based on the search value, but it's basically the "between" functionality. I was never able to figure out why that "else" was in for the between. figured it was maybe something for non mysql database types or something for legacy compatibility.

Take a look at views_handler_filter_numeric.inc at the op_between method (Views 7.x-3.11)

function op_between($field) {
    if ($this->operator == 'between') {
      $this->query->add_where($this->options['group'], $field, array($this->value['min'], $this->value['max']), 'BETWEEN');
    }
    else {
      $this->query->add_where($this->options['group'], db_or()->condition($field, $this->value['min'], '<=')->condition($field, $this->value['max'], '>='));
    }
  }

I just replace $this->value['min'] with my processed value, but its the same query. I'm totally up for removing that "else" if it's really not needed.

paboden’s picture

Status: Needs work » Needs review
Anatolii88’s picture

Hi @paboden,

There are some errors and warnings from Automated project review. Please, check them: http://pareview.sh/pareview/httpgitdrupalorgsandboxphilbo2591875git

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

No: Does not follow the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

* There are some errors regarding the Drupal Coding standards that have to be fixed.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Anatolii88’s picture

Status: Needs review » Needs work
klausi’s picture

Status: Needs work » Needs review

@Anatolii88: there is only one minor coding standard error which by itself is surely not an application blocker, anything else that you found or should this be RTBC instead?

Anatolii88’s picture

Hi @klausi, thank you. There are shown more mistakes that should be mentioned


FILE: /var/www/drupal-7-pareview/pareview_temp/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
 2 | WARNING | Line exceeds 80 characters; contains 86 characters
 4 | WARNING | Line exceeds 80 characters; contains 199 characters
----------------------------------------------------------------------


FILE: ...areview/pareview_temp/handlers/views_handler_filter_numeric_range.inc
---------------------------------------------------------------------------
FOUND 17 ERRORS AFFECTING 12 LINES
---------------------------------------------------------------------------
  13 | ERROR | [ ] Class name must begin with a capital letter
  13 | ERROR | [ ] Class name must use UpperCamel naming without
     |       |     underscores
  18 | ERROR | [ ] Visibility must be declared on method "operators"
  34 | ERROR | [ ] Method name
     |       |     "views_handler_filter_numeric_range::has_extra_options"
     |       |     is not in lowerCamel format
  34 | ERROR | [ ] Visibility must be declared on method
     |       |     "has_extra_options"
  43 | ERROR | [ ] Method name
     |       |     "views_handler_filter_numeric_range::option_definition"
     |       |     is not in lowerCamel format
  43 | ERROR | [ ] Visibility must be declared on method
     |       |     "option_definition"
  56 | ERROR | [ ] Method name
     |       |     "views_handler_filter_numeric_range::extra_options_form"
     |       |     is not in lowerCamel format
  56 | ERROR | [ ] Visibility must be declared on method
     |       |     "extra_options_form"
  91 | ERROR | [ ] Method name
     |       |     "views_handler_filter_numeric_range::op_range" is not
     |       |     in lowerCamel format
  91 | ERROR | [ ] Visibility must be declared on method "op_range"
  99 | ERROR | [x] There must be no space before the colon in a CASE
     |       |     statement
 102 | ERROR | [x] Case breaking statements must be followed by a single
     |       |     blank line
 103 | ERROR | [x] There must be no space before the colon in a CASE
     |       |     statement
 106 | ERROR | [x] Case breaking statements must be followed by a single
     |       |     blank line
 108 | ERROR | [x] There must be no space before the colon in a CASE
     |       |     statement
 125 | ERROR | [x] The closing brace for the class must have an empty line
     |       |     before it
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Time: 81ms; Memory: 6Mb
minnur’s picture

Status: Needs review » Reviewed & tested by the community

@Anatolii88,
@paboden just follows the same naming convention that Views module uses for it's plugins. This is not launch blocker.

Since there are no issues with the module and no security issues other than small formatting issues I feel like this project is ready to be RTBC.

13rac1’s picture

The other called out issues should be fixed where applicable, but aren't blockers to being Fixed.

Jacine’s picture

Unit tests are nice, but requiring them at this stage seems like a bit much.

I would like to see paboden finally get approval here. He's got a lot of potential to be a valuable contributing member of the Drupal community. He also happens to have years of professional Drupal experience and regularly leads training sessions for the community at conferences, which IMO should count for something.

paboden’s picture

Status: Reviewed & tested by the community » Needs review

Hi @Anatolii88, I've updated the module to fix most of the errors from the report. However some of them I don't know if I can fix. Because the View methods they extend are not camel case, fixing them seems to lead to stuff not working anymore. Please advise.

I was able to fix 125, 108, 106, 103, 102, 99, 91 visibility, 56 visibility, 43 visibility, 34 visibility, and 18.

Cheers,
Phil

minnur’s picture

Status: Needs review » Reviewed & tested by the community

All looks good to me. RTBC

minnur’s picture

Can we get some update here. It seems like some modules get approved in 2 months some take more than 6 months even after RTBC.

Jacine’s picture

Agree with @minnur. An update would be great.

Jacine’s picture

Can anyone help?

minnur’s picture

Is there anybody alive out there ? People this is ridiculous.. 7 months later no approval or any kind of update... Other communities, like Wordpress approve projects within a week.

minnur’s picture

Ok, I will pay $30 to a person who will approve this project and get @paboden permission to create D.O projects ... let's see if this will work.

kreynen’s picture

I can't help with this specific issue since I don't have permissions to grant users that role, but I can suggest a work around. Request an organizational account for Chapter 3 if you don't already have one. Request the sandbox project's owner be changed to a vetted user. Promote the project. Request that the project owner be changed again to the organization account (which can't make commits).

In the future create projects as promoted using an existing vetted user then transfer ownership to the organizational account and add the unvetted user. This doesn't help with the PAR backlog for a new developer to "get their wings", but it allows you to create releases and include it in an install profile while you wait.

We've been waiting on #2646234: [D7] User External Invite for > 4 months. This module is deployed in production on 600+ sites at the University of Colorado and while some of the feedback @afinnarn has gotten has been helpful, most of it is just noise from other new contributors trying to get review bonus points to get their own project promoted.

There have been hundreds of attempts to fix this over the years, but the backlog remains and festers. I've chimed in a few times, but I've resigned myself to let this go and put time into improving the things I can like the https://www.drupal.org/project/drupalorg_whitelist and LWG. Using a work around the PAR backlog makes it less frustrating, but I'm guessing that having so many members of the community working around the PAR problem is one of the reasons it never gets fixed.

PAR revamp underway - meanwhile, projects languish in the queue is the most recent summary of the situation I know of. It's much better to discuss in one of those threads since on the rare occasion someone with the required permissions takes a look at the backlog, they are most likely sorting by last updated. Complaining about the PAR process or asking for an update in this issue just puts it at the end of the line :(

mrf’s picture

Using an undocumented backdoor process only available to larger Drupal shops to move this single project forward isn't the goal, we have known about that loophole the entire time this application has been in the queue.

This code has been available on Github for the entire time this application has been in review, getting it publicly available isn't the goal either.

The goal is to allow Phil (who happens to be an extremely prolific module creator) to gain the access he needs to add and promote his many other quality modules on Drupal.org once this one is approved.

It is good to know that there is an undocumented policy for punishing applicants that complain about the process, I'll remember that the next time I encourage someone to go through this process.

kreynen’s picture

@mrf You are preaching to the choir. The only reason I suggested the work around is if we weren't using it, I would have given up on using Drupal.org to host the projects the University of Colorado contributes back long ago. Like many organizations, our primary repo is already GitHub so sharing projects as contrib on Drupal.org is an extra step. Neither PAR nor the workflow required for organizational accounts to create full projects are efficient or enjoyable and discourage contributions. It is really hard to justify the time we spend jumping through these hoops... the result is we contribute less.

https://groups.drupal.org/node/142469#comment-1051678
https://twitter.com/kreynen/status/547163374107099136
#2565989: Proper Project Workflow for Organization Accounts?

I understand your frustration, but I'm not willing to wallow in it. For your own sanity, I really suggest working around the issue. Good luck!

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, Phil!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.