{"action":"opened","issue":{"url":"https://api.github.com/repos/hypothesis/product-backlog/issues/752","repository_url":"https://api.github.com/repos/hypothesis/product-backlog","labels_url":"https://api.github.com/repos/hypothesis/product-backlog/issues/752/labels{/name}","comments_url":"https://api.github.com/repos/hypothesis/product-backlog/issues/752/comments","events_url":"https://api.github.com/repos/hypothesis/product-backlog/issues/752/events","html_url":"https://github.com/hypothesis/product-backlog/issues/752","id":350457626,"node_id":"MDU6SXNzdWUzNTA0NTc2MjY=","number":752,"title":"Make `group`-`organization` relationship NULLable","user":{"login":"lyzadanger","id":439947,"node_id":"MDQ6VXNlcjQzOTk0Nw==","avatar_url":"https://avatars2.githubusercontent.com/u/439947?v=4","gravatar_id":"","url":"https://api.github.com/users/lyzadanger","html_url":"https://github.com/lyzadanger","followers_url":"https://api.github.com/users/lyzadanger/followers","following_url":"https://api.github.com/users/lyzadanger/following{/other_user}","gists_url":"https://api.github.com/users/lyzadanger/gists{/gist_id}","starred_url":"https://api.github.com/users/lyzadanger/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lyzadanger/subscriptions","organizations_url":"https://api.github.com/users/lyzadanger/orgs","repos_url":"https://api.github.com/users/lyzadanger/repos","events_url":"https://api.github.com/users/lyzadanger/events{/privacy}","received_events_url":"https://api.github.com/users/lyzadanger/received_events","type":"User","site_admin":false},"labels":[{"id":490419295,"node_id":"MDU6TGFiZWw0OTA0MTkyOTU=","url":"https://api.github.com/repos/hypothesis/product-backlog/labels/h","name":"h","color":"c2e0c6","default":false},{"id":620337428,"node_id":"MDU6TGFiZWw2MjAzMzc0Mjg=","url":"https://api.github.com/repos/hypothesis/product-backlog/labels/lms","name":"lms","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-08-14T14:37:03Z","updated_at":"2018-08-14T14:37:03Z","closed_at":null,"author_association":"NONE","body":"Summary: We need to alter the current data model involving groups-organizations to support needed functionality for third-party private groups for LMS, but potentially other partner uses as well.\r\n\r\n----\r\n\r\nOur current modeling of `organization` and its relationship to `group`s is brittle and it has become an actual problem (versus a complexity) when it comes to supporting third-party private groups (for LMS and beyond).\r\n\r\nRight now:\r\n\r\n* `organization` is a _required_ (non-nullable) relation on `group` records\r\n* both `group` and `organization` have an authority field\r\n\r\nAll groups must belong to an organization—this is enforced on the DB level.\r\n\r\nIn our _services_ layer, we enforce a match between `group.authority` and `organization.authority`.  This starts becoming more complex because it means that at least one `organization` must exist for every `authority` and that the services need to know which `organization` to slot a given group into.\r\n\r\nWe routed around this complexity (sort of) for first-party private groups by making sure a `Hypothesis` default organization exists wherever the app is running, assigned to the `hypothes.is` authority. We have an `Organization.default` class method that will return that “default” organization. If a group-create service (for any type of group) is invoked without passing an explicit `organization`, the new group is chucked into this default organization. Net result is that first-party private groups all get put into the default, “Hypothesis” organization. The relationship between private groups and the “Hypothesis” organization is contextually meaningless; that relation exists solely because it’s a non-nullable relationship. There is no coherent or semantic “meaning” for a private group being in the “Hypothesis” organization.\r\n\r\nThis breaks down further with third-party private groups. The create-private-group service will attempt to put the new group into the default—Hypothesis—organization. As I mentioned above, there is a validation check here to make sure that the group and organization authorities match, _which they won’t because the default organization is firmly attached to the Hypothesis authority_.\r\n\r\nThe only “workaround” possible in the current setup is to have a default organization available for _every_ authority and to use that organization to put the authority’s private groups into. That means that the service has to know which “default” organization to use, meaning extension of the model or services classes to juggle this. This becomes a deeper and deeper data tangle. It smells.\r\n\r\nThere are two ways I believe this could be alleviated:\r\n\r\n1. Make the `group` -> `organization` relationship nullable. This would be the fullest and most meaningful way to solve this problem set, as the relationships between private groups and their containing organization(s) don’t mean anything.\r\n\r\n2. Remove the `authority` field from `organization` for now. It’s non-normalized that `authority` exists in both places. In this approach, all private groups would be chucked into the default organization (which would no longer be tied down to a single authority). \r\n\r\n----\r\n\r\nAfter discussion with the engineering team, it has been decided that we're going to try for approach 1—making the `organization` relationship on `group` NULLable. This is high-priority as it is on the critical path for LMS work."}}
{"actor":{"display_login":"lyzadanger"}}