Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Add a base for users to subscribe to digests and store that data.
Proposed resolution :
Add a field to the user entity which allows them to select any active digests and subscribe.
Alternative resolutions :
Utilise content entities to hold a one to one relationship between a digest and a user. See #2
Utilise content entities to create a one to many relationship between a digest and users. See #5
Comment | File | Size | Author |
---|---|---|---|
#13 | digest.3154614-13.patch | 14.39 KB | DerekCresswell |
#6 | Digest 3154614.png | 42.78 KB | travis-bradbury |
Comments
Comment #2
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedThere is going to be a good chunk more work done on issue 3152692 first so I will be leaving this on the back burner for now.
Posting the patch mainly for storage. It does not need to be reviewed yet as it is just a WIP.
Comment #3
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedJust going to leave this here as it helped to solve a problem with one of the tests, we need to check if there was a result returned from the subscription query otherwise errors are generated.
Comment #4
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedHere is a subscription entity. I've not added any forms for subscribing to digests here to keep the patches small. I'll make a separate issue for that soon.
Both the digest of the subscription and the users are kept as entity references. I've decided to use a one-to-many relationship model since that is what a digest and users is in real life.
Comment #5
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedIf I attach the patch that is. Haha
Comment #6
travis-bradbury CreditAttribution: travis-bradbury at Acro Commerce commentedI don't get the choice to make one Subscription entity for every Digest entity and use a field for the user reference. They should be config entities or content entities, not both. I tried to visualize this to see if my understanding is correct. I think we have this first one: a Subscription entity, plus a user reference field, which gives us a table that maps digests to users.
https://docs.google.com/drawings/d/19LUgUxNL0NIQAZeAB7rwBpLkJU4OYI4lSm1B...
The other two are other options that avoid duplicating digests as configuration and content. The middle one is a "digest_subscription" entity, which stores the reference between digest and user. It's essentially the same as the entity reference field.
I understand that this was considered:
What's the problem with a large number of entities? If there's a large number of relationships between digests and users, you still have to store that.
The third diagram there is supposed to be a field added to users that references the digest entities they subscribe to. I don't think it makes a difference from the database perspective, but it might be more natural depending on how people are supposed to manage their subscriptions. If the subscription is a field on the user, one could implement the management interface with a field widget, and sites might end up with an easier way to control where that UI appears. I don't know if an entity reference field can reference a configuration entity, but this could be a custom field type instead.
I think you'll want to change the entity's ID and table name, too. "subscription" seems reasonably likely to conflict with another module at some point.
Comment #7
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedMay need to re install module on a test site for these features to work.
One thing I am un-sure about is enabling the field on module install. Currently the configuration will add it to the user but it remains disabled until it is manually enabled (on the manage fields page for users).
Can this be done automatically with the config? I hesitated to put in that bit since it seems like it could overwrite other fields that had been added to the user.
Currently, there is some instruction added to the Readme on how to enable the field. It can be done automatically with this snippet instead :
Though I wanted to get others opinions before adding this. Pair this with one to automatically disable / remove the field when uninstalling.
If a formatter is wanted then it should become a separate issue.
This could use some tests. Having a little difficulty with them though. Will continue if this is ok though.
Comment #8
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedComment #9
joshmillerThese instructions are preferrable to modifying users on install.
Why do we need a widget?
Comment #10
travis-bradbury CreditAttribution: travis-bradbury at Acro Commerce commentedIf you do need the widget, you should consider a new field type. Based on the instruction "Make sure to set the widget of that to "Digest subscriptions" as well.", using the default widget is bad. I'm assuming that using this widget on any other list_string field would be bad too.
You can disarm that footgun by making a new field type (extend the list_string field type if you want) so that your widget is the only option.
Comment #11
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedThe widget just adds the descriptions to each option and in the future other details like when they send. As we speak, I am removing some of the override functions from the widget to clean it up.
The widget will still only work with the digests so perhaps an empty field type would be wise to prevent errors.
Comment #12
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedAs suggested by @joshmiller here is a patch that ditches a widget in favour of using a hook to modify the the wdiget and give it description etc.
I believe this to be the not ideal solution, but am always open to options.
Comment #13
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedWe've decided to stay with the widget. I've removed some unnecessary overriding in the widget, a blank subscription field to prevent errors, and tests to ensure the page is displaying properly.
Comment #14
joshmillerComment #16
DerekCresswell CreditAttribution: DerekCresswell as a volunteer and at Acro Commerce commentedCommitted, thanks.