As discussed in the final comments on #540972: Make legal module OG-aware, I'm considering implementing the per-Group Terms & Conditions as a proper node type, rather than using a custom table, as the Legal module which this is based on does.

I haven't made a firm decision one way or the other here, but just wanted to move the discussion into this project's issue queue, as it seems a more appropriate place to have the conversation. Arguments and considerations welcome, and when I commit the actual code later today, patches too!

Comments

sethcohn’s picture

sethcohn commented

I'd say that making a particular group T&C a node makes 100% sense, since the terms can be tied to a group easily that way, revisions are easy too.
The 'has signed group T&C' might be best done as a table, since you'll want to tie uid (User) to nid (and vid) (Version of T&C). Ideally, you might also store gid there, showing which group it was signed for, so potentially, joining multiple groups could require multiple approvals of the same T&C, once per group, even if the same node T&C covers multiple groups.

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented
Version: » 6.x-1.x-dev
spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

Thanks for your thoughts on this, @sethcohn. My gut feeling is that nodes make sense for the actual T&Cs as well, but there are some sticky bits to this approach I can see as well. For the moment, mainly in the interest of expediency, I've just reused the Legal module's approach and implemented the custom table approach with an extra field for Group node ID. This is probably not as elegant as it could be, but it works.

In particular, if we were to implement the T&Cs as nodes, what happens if such a node is deleted, or the latest revision is deleted after users have already accepted them? How do you handle the case where you revert to an older revision of the node T&C? I'm sure these things can be handled, I just didn't want to deal with them right away :)

sethcohn’s picture

sethcohn commented

1) Node deletion: You can prevent this from happening at all.
2) The latest revision being deleted/etc, That's why I'm recommending storing the vid so you can tell the version someone last signed.
3) Reverting: that would be a admin option (choice of having signed only the current revision, or any revision newer than the current revision)

I understand why the quick answer would be to adopt the legal module's methods, but I think legal is so 'old' that it has some really bad practices in it, which are better done these days in other manners. (revisioning being one of them) [FYI< legal is far from the only module suffering from that age issue. even some core modules (book comes to mind) have legacy approaches that if someone was to write it today, they'd do it more 'drupally']

spiderman’s picture

spiderman
he/they
Primary language English
Location Halifax, NS
commented

@sethcohn: Thanks for the wise words on this. Certainly seems that implementing with nodes is both desirable and feasible. I propose we solidify the current code using the custom table approach for now, work to get a 1.x release ASAP, and then begin working toward a 2.x release where we switch to using the node system.

Have you had a chance to peek at the code I committed yesterday? I intend to test it more thoroughly this afternoon, but would love to get another pair of eyes on it before too much longer.

sethcohn’s picture

sethcohn commented

I'll look at it today...