Skip to content
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

send_push_notifications: 1–based counting, improve messages #3404

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

PeterNerlich
Copy link
Collaborator

Short description

Our `send_push_notifications` management command is giving confusing output.
INFO    integreat_cms.firebase_api.firebase_api_client - <PushNotificationTranslation (id: …, push_notification_id: …, title: …)> sent, FCM id: …
SUCCESS integreat_cms.core.management.commands.send_push_notifications - Successfully sent 0/3 <PushNotification (id: …, channel: …, regions: <QuerySet […]>)>
INFO    integreat_cms.firebase_api.firebase_api_client - <PushNotificationTranslation (id: …, push_notification_id: …, title: …)> sent, FCM id: …
INFO    integreat_cms.firebase_api.firebase_api_client - <PushNotificationTranslation (id: …, push_notification_id: …, title: …)> sent, FCM id: …
SUCCESS integreat_cms.core.management.commands.send_push_notifications - Successfully sent 1/3 <PushNotification (id: …, channel: …, regions: <QuerySet […]>)>
INFO    integreat_cms.firebase_api.firebase_api_client - <PushNotificationTranslation (id: …, push_notification_id: …, title: …)> sent, FCM id: …
INFO    integreat_cms.firebase_api.firebase_api_client - <PushNotificationTranslation (id: …, push_notification_id: …, title: …)> sent, FCM id: …
SUCCESS integreat_cms.core.management.commands.send_push_notifications - Successfully sent 2/3 <PushNotification (id: …, channel: …, regions: <QuerySet […]>)>
SUCCESS integreat_cms.core.management.commands.send_push_notifications - ✔ All 3 scheduled push notifications have been processed.

Successfully sent COUNT/TOTAL–messages are off by one, as we just use for count, pn in enumerate(pns): (0–based).

Additionally, I have ideas how to further improve the messages.

Proposed changes

  • Management command:
    • Change enumeration to start at 1
      This removes uncertainty while interpreting logs.
    • Change messages to print object representations at the very end
      This improves readability of logs, as object representations can be very long and wrap multiple lines. With this change first the event/action is clear (length fixed and easily readable even with many similar log lines), then specifics about the object in question can be read (highly variable character count)
  • Representations of PushNotification and PushNotificationTranslation
    • Change representations to interpolate strings (like title) quoted, not verbatim
      This improves readability as the start and end of the field in the representation is clearly recognizable.
      - <PushNotificationTranslation (id: 12673, push_notification_id: 1877, title: COURSE START ON 27.01.2025, GENERAL INTEGRATION COURSE FROM MODULE 1 (A1-B1), MO-FR, 9:00-12:30, IN AICHACH)>
      + <PushNotificationTranslation (id: 12673, push_notification_id: 1877, title: "COURSE START ON 27.01.2025, GENERAL INTEGRATION COURSE FROM MODULE 1 (A1-B1), MO-FR, 9:00-12:30, IN AICHACH")>

Side effects

  • Any tool depending on the old log output might show unexpected behaviour

Pull Request Review Guidelines

@PeterNerlich PeterNerlich added infrastructure Issues related to the dev or production environment enhancement This improves an existing feature labels Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This improves an existing feature infrastructure Issues related to the dev or production environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant