fix: #students and #coaches return Chapter-specific students and coaches#2579
fix: #students and #coaches return Chapter-specific students and coaches#2579
Conversation
|
I'll give this one a quick test locally, but feels like a nice approach 👍 |
|
Could you add some tests to make sure this stays fixed? |
jonodrew
force-pushed
the
bugfix/fixup-methods-on-chapter
branch
from
78fb645 to
aa9a6d8
Compare
This fixes a bug where the methods returned _all_ of the students and coaches that the user could access Signed-off-by: jonathan.kerr <3410350+jonodrew@users.noreply.github.com>
jonodrew
force-pushed
the
bugfix/fixup-methods-on-chapter
branch
from
aa9a6d8 to
d5751b5
Compare
|
@mroderick done! |
mikej
left a comment
mikej
left a comment
There was a problem hiding this comment.
Hi! The actual change to chapter.rb looks good (had a check of the queries that get generated).
Added a comment about how to include the shared examples in the spec though.
| expect(this_chapter.public_send(method_name)) | ||
| .not_to include(that_member) | ||
| end | ||
| end |
There was a problem hiding this comment.
Good idea to use some shared examples here, but currently these test cases won't actually get run when running chapter_spec.rb. After defining the shared_examples, you also need to add include_examples lines with the appropriate group_name and method_name for students and coaches.
e.g. inside the describe "helper methods... but after the RSpec.shared_examples you can put:
include_examples "group-scoped members", "Coaches", "coaches"
include_examples "group-scoped members", "Students", "students"On doing this, the specs then fail with Validation failed: Group has already been taken so looks like the before results in trying to create the groups twice?
Are you OK to take a look at this as I agree having these tests in place is definitely worthwhile.
Lastly, if you want to check which test cases RSpec is running you can use the "documentation" format which lists the passing tests as well as any failures: bin/drspec -fd spec/models/chapter_spec.rb
3 participants
This fixes a bug where the methods returned all of the students and coaches that the user could access
#coachesquery:"SELECT DISTINCT \"members\".* FROM \"members\" INNER JOIN \"subscriptions\" ON \"members\".\"id\" = \"subscriptions\".\"member_id\" INNER JOIN \"groups\" ON \"subscriptions\".\"group_id\" = \"groups\".\"id\" WHERE \"groups\".\"chapter_id\" = 1 AND \"groups\".\"name\" = 'Coaches'"#studentsquery:"SELECT DISTINCT \"members\".* FROM \"members\" INNER JOIN \"subscriptions\" ON \"members\".\"id\" = \"subscriptions\".\"member_id\" INNER JOIN \"groups\" ON \"subscriptions\".\"group_id\" = \"groups\".\"id\" WHERE \"groups\".\"chapter_id\" = 1 AND \"groups\".\"name\" = 'Students'"