-
Notifications
You must be signed in to change notification settings - Fork 46
Simplify push notification workflow by merging templates and draft status #4052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this big PR. Everything works well and the code is clean. I just have a few thoughts about the flow of "scheduling" and of "copying" - which I could not really grasp from the design plan, but feel should be considered.
1.) I feel that the workflow with scheduling is still not really optimized. Like: if I should only be able to schedule a pn after it has been saved, I feel I shouldnt be able to select "Automatisch senden" and set the date and time during the save process. It should only show when I have the option to schedule. The other way around it is confusing that when I only click on "Speichern", but have selected a date and time, it will save that and show it in the list as "Scheduled for" that date&time.
1a.) If we already have scheduled (actually scheduled, not just saved with a date and time) a notification, is it intended that the notification can be re-scheduled? If yes, maybe the button should show as "Re-schedule"? If no , the "schedule" button should not show
2.) Maybe when creating a copy, the form should open with that newly created copy. (Instead of just appear in the list)
integreat_cms/cms/views/push_notifications/push_notification_list_view.py
Outdated
Show resolved
Hide resolved
|
@hannaseithe
We actually didn't look deeply into the workflow with scheduling at the time of design discussion (as we all forgot that option 😅) The background of the birth of this "saving" step before sending is to gurantee translations in other languages are surely there before user dispatchs the push notification (MT will be connected to the PN form in #2640 ).
Re-scheduling is possible in the current implementation (before this PR) too.
That sounds a good idea 👀 |
Co-authored-by: hannaseithe <hannaseithe@users.noreply.github.com>
853d3aa to
4a41b80
Compare
1a.) I agree with @hannaseithe. @MizukiTemma maybe you can clarify how exactly that would be possible? Unfortunately I can't access this functionality in the Testumgebung.
|
|
@hannaseithe |
jonbulz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the new workflow in my local environment and everything seemed to work as expected. Code looks good to me. Thanks! :)
Short description
This PR introduced a new workflow for the push notification feature.
Proposed changes
draft,is_templateandtemplate_namefromPushNotification(incl. migration)Side effects
Faithfulness to issue description and design
How to test
Resolved issues
Fixes: #2639
Pull Request Review Guidelines