Needs work
Project:
Digest
Version:
1.0.x-dev
Component:
Miscellaneous
Priority:
Major
Category:
Plan
Assigned:
Unassigned
Reporter:
Created:
21 Aug 2020 at 16:52 UTC
Updated:
13 Nov 2020 at 21:43 UTC
Jump to comment: Most recent
This module is nearing a point where it can have a stable 1.0.0 version released.
All issues marking this as a parent are considered essential to the major release and must be resolved before the release (will not include issues marked as fixed before the creation of this issue).
Any discussion for what should be included in 1.0.0 should be housed here.
Comments
Comment #2
derekcresswell commentedComment #3
derekcresswell commentedEssential issues have been rounded up and marked as children.
Up for debate :
Comment #4
derekcresswell commentedToo things that have not been made into separate issues but still worry me are :
a block is empty considering you could be sending out a static block (perhaps the use case there is "hey did you log time?"). Since we don't limit this to views or a specific, like a content type, it seems we may often be sending out blank digests.
Comment #5
derekcresswell commentedComment #6
derekcresswell commentedAdded issue 3168353 for utilising the config entity status.
In reference to #4 empty digests :
The common use case where this occurs is using a view as a block. In this case, views have the a "no results" setting to define what should happen in the case of there being no results. For now, there will be no way to detect an empty digest and avoid sending it altogether. Users should set up something like a fall back for a view to display.
In the future, if this proves to be an awful idea, we can add in an event that allows developers to override the
shouldSendor the queuing them. This would allow for a custom check, for instance "is there new content", and stop a digest from sending.That was problems itself and would likely only work in a rather static / small site seeing as that would presumably apply to all digests or you'd need to reference the name of it (bad practice). But that all comes later.
Comment #7
derekcresswell commentedTwo notes
CSS, after using this on a real site, using Swiftmailer and the new CSS inlining features in 2.0 there should be no problem if you use the right tools.
If we want that ability to cancel digests, I'm going to suggest just making a 'send event' that is fired when a digest tries to send. Give it an option 'cancel' and we should be golden.
Comment #8
derekcresswell commentedThis issue will contain a patch for updating the readme and such (anything pertaining to early development warnings) Once this is ready for a stable release.
A release candidate has been made : https://www.drupal.org/project/digest/releases/1.0.0-rc1
After a little further testing this should be ready for the big leagues.
Comment #9
derekcresswell commentedBack with a continuation of my thoughts in #6
Stop digests from sending with a hook, event, or we could do a plugin.
This would be a fairly large change and might not be able to be here until v2 unless it's really wanted here. I don't know if this would be replacing the schedule or adding onto it.
Adding onto it:
We allow each digest to specify a list of "conditions" to send out during. These conditions are plugins that allow for us to easily add extra requirements for sending. For instance, I imagine a "New Content" plugin which allows you to choose a time frame, content type to check for, and how many new ones there should be. Then this will not send the digest if there is no new content for it.
Replacing:
This is more of a wild thought, I just wonder if it would then be smart to streamline the schedule into a plugin as well. If so, it'd probably be wise to make this a default on every digest. But theoretically you might want to have a (not quite) digest that sends out as soon as a piece of new content is detected.
With this approach, everything is kept stream lined and some responsibility is lifted from the digest class. It does allow for maybe a more general use case of the digest (somewhat sending out single pieces of content or similar).
Comment #10
gnumatrix commentedDefinitely V2 kinds of things I think?
Enabling or disabling the digest should suffice for allowing a digest to be temporarily suspended. If there is no content, and you're using a view to generate the email, there is a mechanism in the view to put in something like "Sorry, there is no content at this time". Good opportunity to maybe include other content in the email as well, just in case. May be undesirable to send these out, but if it is a common occurrence, then maybe the admins can adjust the frequency of the digest, not the module?
For the fancier scheduling, I dunno, I very much like the way it is. And it is a digest after all - I want it to be a collection and on a set schedule. Other modules can do the work of dealing with event-based notifications. Only request is to make it more obvious in the digest list as to when the next scheduled digest is expected.
I think Digest is pretty great the way it is now, just need to get it past your 1.0 milestone :-)
Comment #11
derekcresswell commentedThe problem with enabling and disabling is that digests are config so that's changing your code base often.
I just wanted to jot down the idea so it would not be forgotten. It'll be made into a separate issue later.
The actual scheduling mechanics of the cron expression wouldn't change in the users eyes.
When it'll send for users is covered here https://www.drupal.org/project/digest/issues/3162691 which will likely be a 1.1 addition. I know there was mention of adding the same to the digest list. Same story there.
Comment #12
derekcresswell commentedI'm going to mark this as NR and if community members or myself (in a little while) change it over to RTBC I'll consider that good enough for the full release.
Comment #14
derekcresswell commentedApplied for security policy, https://www.drupal.org/project/projectapplications/issues/3174609
Hopefully that doesn't take too long, D.O seems to warn against making a full release until we have that. So I will hold back for a moment.
Repo is ready and tagged. The commit just removed the active development warning from the readme.
Comment #15
derekcresswell commentedComment #16
derekcresswell commentedComment #17
derekcresswell commentedI am going to include a new feature in the release. Setting back to needs review. But we will need to make a new RC (might be better as a beta though).
Adding a new issue to the related.
Comment #18
derekcresswell commentedSwitching back to NW for a while. Some new issues have been brought up. This means that there will need to be a new RC before full launch.
Comment #19
derekcresswell commentedComment #20
derekcresswell commentedMight look to make a beta in the next few days. Seemed to jump the gun a bit on the RC, but honestly I love finding issues because then I can make this better.
Comment #21
derekcresswell commentedComment #22
derekcresswell commentedWe have gained security coverage! #3174609: [D9] Digest
I have also made a new beta release as the current RC-1 is not a good image of what has now been done to the module. I know there are a few important issues still up as NR but I think it is necessary to get a more current release up for now. Also aware it is somewhat bad progress to regress from RC to beta but hey, what are ya gonna do eh?
Comment #23
gnumatrix commentedCongratulations on the security coverage.