Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Ok, I have read:
http://drupal.org/node/1497016
http://drupal.org/node/1342632#comment-5450624
and og.test, but I still couldn't get my head around this other group thing, could someone provide a use case for this field?
Thanks
Comment | File | Size | Author |
---|---|---|---|
#11 | 1519702-hidden-gids-with-tests.patch | 11.18 KB | jrao |
#9 | 1519702-hidden-gids-with-tests.patch | 11.19 KB | jrao |
#7 | 1519702-hidden-gids-with-tests-7.patch | 11.65 KB | amitaibu |
#6 | 1519702-hidden-gids-with-tests.patch | 11.57 KB | jrao |
#3 | 1519702-hidden-gids.patch | 3.49 KB | amitaibu |
Comments
Comment #1
amitaibuFor example, as admin you edit a node created by another user. If the node is assigned to a group that doesn't belong to the admin, that group will appear in the "other groups". Remember, the group-audience field options are per user (because each user is subscribed to different groups).
Comment #2
jrao CreditAttribution: jrao commentedOk, played this for a while, let me know if my understanding is correct:
1. This secondary field is only used for editing group content
2. When editing, the groups associated with the content is divided between primary field and secondary field, primary field will only show groups the current user belongs to
3. I'm guessing this design is chosen so that when a user cannot see all the groups the content associated with, he can still save the content and the save won't wipe out the groups he cannot see in the primary field.
Comment #3
amitaibu> I'm guessing this design is chosen so that when a user cannot see all the groups the content associated with, he can still save the content and the save won't wipe out the groups he cannot see in the primary field.
Indeed, finally I got to implementing this -- Probably needs testing - @jrao, any chance you'll add some tests... ;)
Comment #4
jrao CreditAttribution: jrao commentedI'd like to help, but I'm not sure I understand the purpose of this patch, I thought the primary/secondary implementation is already completed, i.e. if I'm admin1 of group1, and I edit node1 which belongs to group1 and group2, the edit page will show group1 in primary field, group2 in secondary field, and after save node1 is still linked to group1 and group2. What is the scenario this patch will fix?
Comment #5
amitaibuPatch fixes the case where a non admin user edits the node, but they don't have access to the secondary field. The patch uses the "invisible" field to keep the "hidden" group ids
Comment #6
jrao CreditAttribution: jrao commentedOk, let's see if this works.
Comment #7
amitaibuThis is same patch form #6, only removed all the tabs from og.test, so it's easier to review. Review will follow.
Comment #8
amitaibuThat's really great of you! Some nitpicking;
It's a leftover, lets remove this.
No need to reference issue.
There's a lot of setup going here, maybe we can add some PHPdocs to explain a bit what were doing.
Missing dot in end of line. Also make sure that comments don't pass the 80chars line.
Let's move this function to be the last one in the test Class.
Isn't it strict by default? Anyway, this can probably be moved to the setup part.
I think we don't need this assert - we have other tests that should cover it.
Instead of escaping (e.g. doesn\'t) we can start text with
"
Since we are checking in next lines og_is_member(), I think we can remove these asserts.
Maybe better "...field isn't accessible by user."?
Comment #9
jrao CreditAttribution: jrao commentedOk, should be fixed now.
Comment #10
amitaibuI think your editor is still using tabs, and you have extra white space (use dreditor to see them).
Comment #11
jrao CreditAttribution: jrao commentedOk, white space and tab should be fixed now. Could I run dreditor against local patch files?
Comment #12
amitaibu> Could I run dreditor against local patch files?
No, but you can setup your Eclipse/ Aptana to trim whitespace, and tabs -> spaces.
Thanks for your great work on the tests -- you received your first commit credit in OG. I hope to give you more... :)
Comment #13
jrao CreditAttribution: jrao commentedOk, no problem, didn't know Eclipse could remove trailing whitespace, thanks for the tip.
Comment #14
amitaibuSlightly related: #1541672: Don't check "create" access in OgSelectionHandler::buildEntityFieldQuery()