Update Documentation on Scopes

[jonmchan]

With the update to make scope method utilize kwargs, is the documentation still accurate?

I originally had code similar to the following:

  # Define custom scopes for the index view
  scopes do
    scope :all
    scope :published, { default: true }, -> { Post.published }
    scope :drafts, -> { Post.unpublished }
  end

This broke after the kwargs update. I have to write it as follows for this to work:

  # Define custom scopes for the index view
  scopes do
    scope(:all)
    scope(:published, default: true) { Post.published }
    scope(:drafts) { Post.unpublished }
  end

Yes, I could use mixed formatting, like:

  # Define custom scopes for the index view
  scopes do
    scope :all 
    scope(:published, default: true) { Post.published }
    scope :drafts, -> { Post.unpublished }
  end

but this looks totally inconsistent between each call...

Should the docs be updated to utilize scope(:published, default: true) { Post.published } format such that you can both pass parameters and have a block in the same call?

Leaving off the () will not work when passing a block (scope :published, default: true { Post.published } is invalid)

I think this looks the most consistent:

  # Define custom scopes for the index view
  scopes do
    scope(:all)
    scope(:published, default: true) { Post.published }
    scope(:drafts) { Post.unpublished }
  end

Definitely welcome feedback on how to pass kwargs and the block in the same call and make it look consistent.

[jonmchan]

Well, there's another option:

scope :published, -> { Post.published }, default: true 

seems to work as well. Not sure this is the most intuitive to me to see the block in the middle of the arguments?

[spohlenz]

The syntax used in the README example looks to still be accurate, even with the change to kwargs.

scope :published, { default: true }, -> { Post.published }

I have a feeling that your scope proc would have been ignored pre-kwargs changes, as the proc must be passed as a block unless it is the second positional parameter.

I definitely agree regarding the aesthetic downsides of the mixed formatting example. Unfortunately in these situations, Ruby makes it tricky to be consistent.

scope :published, -> { Post.published }, default: true 

This alternative ordering was inspired by Rails' associations declaration DSL. My personal preference is to use this option for simple, single-line scopes and use the block form (with do...end) for complex multi-line scopes.

[jonmchan]

@spohlenz - you are correct. The README as-is has no errors. The combination case of passing scope and marking something as default is however not covered in the README.

I assume that it may be a common pitfall for a developer that needs both default and a defined scope to combine them as I mentioned above (scope :published, { default: true }, -> { Post.published }). This results in a ArgumentError: wrong number of arguments (given 3, expected 1..2) error at this point. Adding documentation to cover both default and passing a block for the defined scope might be helpful and save a few minutes or hours of frustration for a developer trying to make this work (like me 😅).

However, the fact that this issue even exists can serve as documentation. I'm sure we mentioned enough keywords for it to show up in google. If you'd like to just close this issue, that would be fine as well. :)