Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveStorage matchers #1102

Closed
a14m opened this issue May 9, 2018 · 7 comments
Closed

ActiveStorage matchers #1102

a14m opened this issue May 9, 2018 · 7 comments

Comments

@a14m
Copy link

a14m commented May 9, 2018

What is the consensus about adding macro matchers for the following?

  • has_one_attached (from ActiveStorage Rails 5.1)
  • has_many_attached (from ActiveStorage Rails 5.1)
@a14m a14m changed the title ActiveStorage and Paperclip matchers ActiveStorage matchers May 9, 2018
@mcmire
Copy link
Collaborator

mcmire commented May 10, 2018

I can't speak for the rest of the team, but I personally haven't used ActiveStorage yet. But if you're finding that you're using it a lot and you've worked out some matchers locally for them, and you'd be willing to be a sort of "champion" for this down the road, then we'd totally welcome the new matchers into this gem.

@a14m
Copy link
Author

a14m commented May 10, 2018

Great, it's a new project and we opted in for using ActiveStorage, I'll try to work on the matcher later and keep this issue up to date with the progress (and reference to the commits)...
Still not quite familiar with ActiveStorage and it's moving parts, so it'll be good to have input here from other people who are interested.

@katelovescode
Copy link

We opted in to ActiveStorage and would love this as well.

@mcmire
Copy link
Collaborator

mcmire commented Sep 13, 2018

@guialbuk What do you think if we closed this for now? I'm not sure how this would work, and considering there's still a bunch of bugfixes in the backlog to merge in first, I'm not sure if we'd be able to get to this any time soon.

@schadenfred
Copy link

schadenfred commented Dec 4, 2018

Without weighing in on whether or not adding this macro is a good idea, you can still use the has_many matcher to test ActiveStorage, given the following active record class in app/models/document.rb:

class Document < ApplicationRecord
  has_many_attached :documents_versions
end

Using Minitest::Spec in test/models/document_test.rb you should still be able to do:

describe Document do 

  specify "has_many" do

    must have_many(:documents_versions_attachments)
  end
end

If you'd like to write a custom macro, open up a rails console and use this for inspiration:

Document.reflect_on_all_associations(:has_many).map(&:name)

Good luck!

@abhchand
Copy link

Hey all -

I just opened a PR with activestorage to formally add validations to the library. See rails/rails#35390

The validations cover content_type and size. Validating presence is a bit difficult (explained in the PR) but I hope to add it at some point.

I love the shoulda-matchers library (huge thanks to everyone who has helped build it). I think it would be great to add matchers in here.

If I'm lucky enough to get that merged I'll consider adding matchers here if time permits 🤞 In the meantime any support on that PR (code review, a thumbs up, etc...) would help nudge it forward too.

Thanks!

@mcmire
Copy link
Collaborator

mcmire commented May 6, 2020

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

@mcmire mcmire closed this as completed May 6, 2020
StefSchenkelaars added a commit to StefSchenkelaars/shoulda-matchers that referenced this issue Jul 6, 2020
StefSchenkelaars added a commit to StefSchenkelaars/shoulda-matchers that referenced this issue Jul 6, 2020
StefSchenkelaars added a commit to StefSchenkelaars/shoulda-matchers that referenced this issue Jul 24, 2020
@mcmire mcmire mentioned this issue Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants