rgsoc-teams
rgsoc-teams copied to clipboard
Issue 1012 - Supervisor unable to see team member email if hide_email: true
Related issue #1012
While this does result in a passing test, giving the supervisor the ability to see a team member's email regardless of hide_email flag being turned on, it also allows the supervisor to read all attributes for that team member. This is likely a result of me not fully understanding the codebase yet. I tried to emulate some of the other methodology/syntax present in the models/ability.rb file but was unsuccessful. The strangest part for me is that the 'other_user' (models/ability.rb:51-52) is only an email and not actually a User, hence the find_by_email. If this solution is unacceptable I would be happy to continue working on the issue until I have found the best option. Thanks!
I ran the full test suite before and after making this change. All tests pass with the exception of one pending, and one failing test(spec/models/season_spec.rb:208) which was failing when I first cloned the repo.
@klappradla @anubiskhan Great that you have this thing going! I think I can shed a light on some of the mysteries, but I won't be able to have a good look until somewhere this weekend.
@anubiskhan I can see how this must be confusing... The issue was not written with a new contributor in mind. My bad, sorry! Let me give you a bit more context.
In previous PR's, we worked on rearranging the Abilities, and this supervisor's ability has not been updated yet. This is the behaviour we want to have:
- For users who did not set their email address hidden:
- email address is available (in views) to all confirmed other users (not for guest users).
- For users who did set their email address hidden:
- email addres is not available to any user, except:
- admin users
- the user herself (for now: you can make it only when that user herself is confirmed)
- when user is a student in a current team: the team's supervisor
That is currently managed with the old code (marked FIXME): https://github.com/rails-girls-summer-of-code/rgsoc-teams/blob/6bfd1b83c2f7f760ab07b33426423e3c968089e1/app/models/ability.rb#L66-L74 And also present in the feature tests for guest and confirmed users.
The first part of this issue (= the first checkbox in the description of the issue) is about cleaning up this old code.
- [ ] As you probably saw, we have abilities for confirmed users, and the
can :read_mailis already implemented there for other users. - [ ] The rule to be able to see your own email should be added.
- [ ] Check if the admin rules cover this
can :read_emailas well - [ ] Add abilities for supervisor's to the
#team memberspart (where you already added your code 👍 ) with thecan: read_emailrule. And thecan :read, :users_infoYou can use the existing method in https://github.com/rails-girls-summer-of-code/rgsoc-teams/blob/6bfd1b83c2f7f760ab07b33426423e3c968089e1/app/models/ability.rb#L123
With that in place, all the lines 66-74 (the code snippet above) can be deleted - not only the rule in line 70 - sorry, I can see how that was not clear in the description.
With the test update proposed by @klapraddla those test should reflect this behaviour.
The can :read_email is a bit different than 'normal' abilities, as @klappradla said, because it is kind of a view helper.
The view helper is used in the User#show view (the user's profile page) and the Community page:
https://github.com/rails-girls-summer-of-code/rgsoc-teams/search?l=Slim&q=can%3F+%3Aread_email
There are some more steps in the issue 1012 description , but let's see if the tests pass with this first part first. ;-) Unless you are totally discouraged, and want to take on a different issue. Either way, we are happy to help you out, and please let us know your thoughts!!
Happy coding!
This looks great, I will take another swing at it. Thanks for all of the feedback!