group-income icon indicating copy to clipboard operation
group-income copied to clipboard

Notifications sent at wrong time/day

Open dotmacro opened this issue 10 months ago • 10 comments

Problem

On March 24th, I received a notification that a new distribution period had started. However, the distribution period shouldn't have rolled over because it hadn't been 30 days. Also, TODOs indicated payment was due on March 31, making it unclear from the UI whether or not the distribution period had rolled over. image

Maybe a few hours later, I received a second notification saying there was a week left in the period, which was correct. However, this meant that the notifications were inconsistent because the period cannot have restarted while also still having a week remaining. image

As far as I remember, I did not change my system clock during this period.

So there are two issues that may or may not have the same/related cause:

  1. Notification incorrect: I received a notification at the incorrect time
  2. Notifications inconsistent: I received an otherwise correct notification that contradicted another notification

Solution

Notifications should be sent at correct time. Notifications should also be internally consistent. Fix it.

dotmacro avatar Apr 01 '24 06:04 dotmacro

image I just received a notification that a new distribution period had started, even though the current distribution period started approximately 4 days ago, and even though a previous notification incorrectly said the new distribution period started approximately a week and a half ago (March 24).

This notification was sent not long after I completed a TODO, so perhaps the notification is incorrectly triggered when the payment that locks the distribution is made.

dotmacro avatar Apr 05 '24 04:04 dotmacro

@taoeffect Can you confirm whether the distribution-locking payment for last period was made on March 24th (pacific time)? If so, that's two months where the "A new distribution period has started" notification was sent at the wrong time, potentially triggered by the distribution-locking payment.

That's a good time to send a notification, but the wording there should be specific to the distribution being locked, rather than incorrectly saying a new period has started.

dotmacro avatar Apr 05 '24 04:04 dotmacro

@taoeffect Can you confirm whether the distribution-locking payment for last period was made on March 24th (pacific time)? If so, that's two months where the "A new distribution period has started" notification was sent at the wrong time, potentially triggered by the distribution-locking payment.

Yep, I did send a payment then, it might have been the first one.

That's a good time to send a notification, but the wording there should be specific to the distribution being locked, rather than incorrectly saying a new period has started.

👍

taoeffect avatar Apr 05 '24 08:04 taoeffect

The "distribution locked" notification text should be: "First payment sent. Distribution is now locked."

Note: A fix for this bug may overlap with the UI PR for #1914

dotmacro avatar Apr 08 '24 04:04 dotmacro

Hi @taoeffect , Have started investigating/working on this issue. I have re-checked the logic that governs whether to send that particular notification or not (which is here)

return currentPeriod &&
    comparePeriodStamps(dateToPeriodStamp(new Date()), currentPeriod) > 0 &&
    !myNotificationHas(item => item.type === 'NEW_DISTRIBUTION_PERIOD' && item.data.period === currentPeriod)

And it has been working correctly when I tried this in my local development (by changing around the clock on my machine).

Q. My question is, has the group's distribution period been changed in the last few weeks? If not, can I please get the actual .db files of the live group for my further investigation? I need to check what value of item.data.period each duplicate notification is holding and I guess that will reveal what's causing the issue.

Cheers,

SebinSong avatar Apr 10 '24 01:04 SebinSong

@SebinSong sure, I'll reply on Slack

taoeffect avatar Apr 10 '24 01:04 taoeffect

@taoeffect Left this comment on the related slack thread too but,

These two seemingly duplicated notifcations

actually are for two consecutive/different distribution periods, if we look at the data.

The solution I would suggest is, to display what distribution period the notification is saying.

e.g) A new distribution period (1st, Apr, 2024) has started. Please check ....

What is your thoughts?

cc. @dotmacro

SebinSong avatar Apr 10 '24 04:04 SebinSong

@SebinSong Yeah, if that's the problem, then that is an acceptable solution 👍

I'm not yet convinced that's what happened, but let's try this anyway because if it turns out to be something else this method will help us track down the real problem.

taoeffect avatar Apr 10 '24 15:04 taoeffect

@taoeffect agreed!

SebinSong avatar Apr 10 '24 20:04 SebinSong

These two seemingly duplicated notifcations actually are for two consecutive/different distribution periods

How is it possible to have two consecutive distribution periods spaced ~7 days apart if periods are 30 days?

The solution I would suggest is, to display what distribution period the notification is saying. e.g) A new distribution period (1st, Apr, 2024) has started. Please check ....

If we include the date, it should be specified whether that's the start date or the due date. How about: "A new distribution period has started, ending $date. Please check Payment TODOs."

dotmacro avatar Apr 12 '24 03:04 dotmacro

@dotmacro

How is it possible to have two consecutive distribution periods spaced ~7 days apart if periods are 30 days?

That's a part that remains mysterious now. As you can see from below screenshot, the timestamp value for the period '2024-03-02' is April 3rd, which is wrong. But my logic that decides whether to emit a notification or not actually has a line that prevents this exact case from happening and throughout all my attempts to reproduce this issue so far, I've only seen that logic is actually working properly.

image (3)

I will leave this issue for the time being to work on other tasks/projects assigned to me but will do keep an eye on this one and check if this continues to happen in both the live staging website and in the local development environment.

cc. @taoeffect

SebinSong avatar Apr 13 '24 02:04 SebinSong