Prevent PNs from being sent without user using Send button

[MizukiTemma]

Short description

This PR fixes the bug that PNs which was only saved and not yet "sent" was sent by the management command cron job.

Proposed changes

- Add a flag whether user clicked "Send" button (set ready_to_be_sent to True).
- Check for ready_to_be_sent = True when the management command is collecting failed (one-time) PNs
- Do not care ready_to_be_sent, as scheduled PNs are to be considered as "ready" (agreed here )

• Make management command check only scheduled PNs

Side effects

• I hope none......

Faithfulness to issue description and design

There are no intended deviations from the issue and design.

How to test

• Create a PN (X)
• Save (click "Save" button)
• Create another PN (Y)
• Save (click "Save") and send (Click "Send")
• ./tools/integreat-cms-cli shell and set sent_date of Y to None
• ./tools/integreat-cms-cli send_push_notifications
• See only Y is sent, X is not sent

Use "Testumgebung" or a region which does not exist in the prod system

Resolved issues

Fixes: #4080

---

Pull Request Review Guidelines

[PeterNerlich]

I feel like it would be cleaner to handle a click on send as if the news was scheduled for right now:

• User schedules message:
  • User selects date in the future
  • User clicks schedule button
  • send_push_notifications (running periodically) sends a notification IF the scheduled date is in the past AND the news is not marked as already sent
• User sends message directly:
  • User clicks send button
  • News gets scheduled timestamp set to the current timestamp
  • send_push_notifications (running periodically) sends a notification IF the scheduled date is in the past AND the news is not marked as already sent
• User saves message:
  • User clicks save button
  • Scheduled timestamp remains as None
  • send_push_notifications thus won't send it

I realize that this doesn't work if users want to just set a date, edit content some more later and not have a notification sent out when the date passes and they didn't get around to their finishing touches and explicitly saying "this is ready now". Is this relevant to us?

If we don't want this reliance on whether the scheduled timestamp is None, my personal opinion would be that a draft property still makes the most sense and should be brought back – even if you won't notice in the frontend. ready_to_be_sent is verbose enough in a sense though

[PeterNerlich]

Another way to look at it (I hope I imagine the workflow correctly): When the user clicked send, the news is marked as ready to be sent. When they edit it and click save, in my opinion this should not change this ready-to-be-sent status. Currently it marks it as not ready again I think

[MizukiTemma]

Summary of the long discussion

• Do not re-send not-scheduled push notifications (because warning after "Send" button is enough)
• Re-send only scheduled failed push notifications (because users probaly do not realise soon when it fails)

[osmers]

Summary of the long discussion

• Do not re-send not-scheduled push notifications (because warning after "Send" button is enough)
• Re-send only scheduled failed push notifications (because users probaly do not realise soon when it fails)

Just to clarify - not scheduled means messages that were intended to be sent immediately, so as per @PeterNerlich suggestion they are still scheduled but for immediately, right? Then why can't we resend them as well? They'll have a date in the past, just as the scheduled notifications.
Any PN that should not be send, shouldn't have any date attached to it, right?

[MizukiTemma]

Summary of the long discussion

• Do not re-send not-scheduled push notifications (because warning after "Send" button is enough)
• Re-send only scheduled failed push notifications (because users probaly do not realise soon when it fails)

Just to clarify - not scheduled means messages that were intended to be sent immediately, so as per @PeterNerlich suggestion they are still scheduled but for immediately, right? Then why can't we resend them as well? They'll have a date in the past, just as the scheduled notifications. Any PN that should not be send, shouldn't have any date attached to it, right?

As "not-scheduled PNs I meant PNs users have sent immediately ("Send" button).

I did not implement "re-send per scheduling" for failed not-scheduled PNs, as error messages are shown to users (and users see it). The problem was that users do not recognise an PN failed if it is scheduled (because users are probably not in the CMS system), isn't it?

[osmers]

Summary of the long discussion

• Do not re-send not-scheduled push notifications (because warning after "Send" button is enough)
• Re-send only scheduled failed push notifications (because users probaly do not realise soon when it fails)

Just to clarify - not scheduled means messages that were intended to be sent immediately, so as per @PeterNerlich suggestion they are still scheduled but for immediately, right? Then why can't we resend them as well? They'll have a date in the past, just as the scheduled notifications. Any PN that should not be send, shouldn't have any date attached to it, right?

As "not-scheduled PNs I meant PNs users have sent immediately ("Send" button).

I did not implement "re-send per scheduling" for failed not-scheduled PNs, as error messages are shown to users (and users see it). The problem was that users do not recognise an PN failed if it is scheduled (because users are probably not in the CMS system), isn't it?

Yes, that is definitely the problem - I was wondering why we can't implement the same procedure for PNs that fail when trying to send automatically. I understand that we can simply show the error message and ask users to re-send. Is there an advantage in doing that as opposed to using the same process as for scheduled PNs?

[MizukiTemma]

Summary of the long discussion

• Do not re-send not-scheduled push notifications (because warning after "Send" button is enough)
• Re-send only scheduled failed push notifications (because users probaly do not realise soon when it fails)

Just to clarify - not scheduled means messages that were intended to be sent immediately, so as per @PeterNerlich suggestion they are still scheduled but for immediately, right? Then why can't we resend them as well? They'll have a date in the past, just as the scheduled notifications. Any PN that should not be send, shouldn't have any date attached to it, right?

As "not-scheduled PNs I meant PNs users have sent immediately ("Send" button).
I did not implement "re-send per scheduling" for failed not-scheduled PNs, as error messages are shown to users (and users see it). The problem was that users do not recognise an PN failed if it is scheduled (because users are probably not in the CMS system), isn't it?

Yes, that is definitely the problem - I was wondering why we can't implement the same procedure for PNs that fail when trying to send automatically. I understand that we can simply show the error message and ask users to re-send. Is there an advantage in doing that as opposed to using the same process as for scheduled PNs?

@osmers
Do you mean there is any advantage in showing an error messsage and not re-send failed one-time (immediate/not-scheduled) PNs?

• I understood your replies as "Error message is maybe enogh, go with it and do something else (for example re-sending) if error message it not enough". So I didn't do anything with re-sending.
• It is technically not cheap to distinguish whether user clicked "Save" or "Send" at the last time. We can of course do a renovation but I don't think it's worth the effort: actually fail cases require actions on user side: a translation is missing, user does not have permission or the PN is already sent/archived ("Send" button is shown incorrectly), setting for PN is not appropriate (user has to report).
• I am personally not fan of turning failed one-time (immediate/not scheduled) PNs to scheduled PNs (for re-sending porpose), as then some date&time appears in the column "PLANNED SEND DATE" in the PN list while user did not do such action.

[osmers]

• I am personally not fan of turning failed one-time (immediate/not scheduled) PNs to scheduled PNs (for re-sending porpose), as then some date&time appears in the column "PLANNED SEND DATE" in the PN list while user did not do such action.

Thanks for pointing that out - that was not obvious to me and is a good argument for why not to deal with these PNs in that way.

Let's come up with a unified approach during our call :)