Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Allow accessible scope exclusion for blank value #7

Merged
merged 1 commit into from
Sep 11, 2013

Conversation

ersatzryan
Copy link
Contributor

No description provided.

@@ -28,6 +28,8 @@ def periscope_call(chain, scope, param)

if options[:boolean]
values.first ? chain.send(method) : chain
elsif options[:ignore_blank]
values.first.empty? ? chain : chain.send(method)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not depend on Active Support here? Is that why you used empty??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was bombing on one of the adapters so i just went with empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't investigate to far, just figured i couldn't depend on ActiveSupport to be there

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we're okay with:

 > " ".blank?
=> true
 > " ".empty?
=> false

@laserlemon
Copy link
Owner

How you have it now, you can't use the ignore_blank option and pass in arguments.

scope_accessible :age, ignore_blank: true

def self.age(age)
  where(age: age)
end

@laserlemon
Copy link
Owner

I also don't see any reason why ignore_blank couldn't work alongside the boolean option. Right now, boolean scopes are only excluded if the value is nil but not if the value is "". Combining the two options could make that happen.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 275a077 on ersatzryan:ignore-blank into * on laserlemon:master*.

@ersatzryan
Copy link
Contributor Author

looks like that error is a database_cleaner issue... might need to tweak the gemfiles in the matrix to get a good version of it

@ersatzryan
Copy link
Contributor Author

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d590fe0 on ersatzryan:ignore-blank into * on laserlemon:master*.

@laserlemon
Copy link
Owner

♻️ 🙏

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d99c24d on ersatzryan:ignore-blank into 572a6a2 on laserlemon:master.

laserlemon added a commit that referenced this pull request Sep 11, 2013
Allow accessible scope exclusion for blank value

[ci skip]
@laserlemon laserlemon merged commit c61370f into laserlemon:master Sep 11, 2013
@ersatzryan ersatzryan deleted the ignore-blank branch September 12, 2013 02:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants