Slack: Check if muteUnfurlPromptsUntil timestamp is after now and don't show this setting if that is the case

[wilhelmklopp]

https://github.com/integrations/slack/blob/7e6177f212dbdf2233eef9f7d3ef8d1b98e671e9/lib/messages/user-settings/index.js#L39

[stale[bot]]

Is this still relevant? If so, just comment with any updates and we'll leave it open. Otherwise, if there is no further activity, it will be closed.

[wilhelmklopp]

yep

On Fri, Jul 13, 2018 at 7:25 PM stale[bot] [e-mail] wrote:

Is this still relevant? If so, just comment with any updates and we'll
leave it open. Otherwise, if there is no further activity, it will be
closed.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/integrations/slack/issues/575#issuecomment-404805778,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHXHLlaBIqDYTZgG8BW-SkfNuaTN85-mks5uGIOogaJpZM4T9kLC
.

[doms]

@wilhelmklopp 👋

I want to try and tackle this, but I just want clarification. When you say _check if muteUnfurlPromptsUntil timestamp is_ after now, do you mean something like:

if ((this.muteUnfurlPromptsUntil > moment.unix()) || this.muteUnfurlPromptsIndefinitely) {
  // ignore this, don't show setting...
}

(basically I just want to find out what I'm doing in that conditional in your original comment)

[wilhelmklopp]

@doms 👋

Ah good catch! I think I mean exactly the opposite. We don't want to show the setting if the current unix timestamp is higher than the this.muteUnfurlPromptsUntil value and this.muteUnfurlPromptsIndefinitely is not set.

Since that would suggest that a user has decided to mute for say 24h, but we still give them the option in the settings to "remove the mute" even though that has already happened.

Let me know how you get on! I believe there is some _time-related_ oddness in some of the tests around this logic.

[doms]

@wilhelmklopp cool, thanks for the clarification!

So rather than the snippet in my previous comment, instead we would want:

if ((this.muteUnfurlPromptsUntil < moment.unix()) && !this.muteUnfurlPromptsIndefinitely) {
  // don't show setting...
}

where this.muteUnfurlPromptsUntil is _less than_ moment.unix() and this.muteUnfurlPromptsIndefinitely is negated since we are expecting it to not be set? 🤔 (or maybe I'm overthinking this and should just play with it and observe the behavior... 😄)

[wilhelmklopp]

Yep, that's exactly right 👍

(or maybe I'm overthinking this and should just play with it and observe the behavior... 😄)

I don't blame you haha! It's an unreasonable amount of effort to get this running locally 🙈

[doms]

@wilhelmklopp

I think I'm almost ready to push this up, I just need to fix the tests! You were right about there being some _time-related_ oddness 😄

Do I need to update the failing snapshots? I did some digging through the jest snapshot testing docs because no matter what I did, the tests didn't pass, then I realized, maybe the behavior is right, but the snapshots need to be updated! (since its expecting the _old_ value in the *-test.js.snap file)

I'm not sure if this could be remedied with just changing the logic of the tests, or if updating the snapshots is a must. 🤔

[wilhelmklopp]

Happy to take a look at a WIP PR of what you have right now on Monday 👍

On Fri, Sep 7, 2018, 22:01 Dominic [e-mail] wrote:

@wilhelmklopp https://github.com/wilhelmklopp

I think I'm almost ready to push this up, I just need to fix the tests!
You were right about there being some time-related oddness 😄

Do I need to update the failing snapshots? I did some digging through
the jest snapshot testing docs
https://jestjs.io/docs/en/snapshot-testing.html because no matter what
I did, the tests didn't pass, then I realized, maybe the behavior is right,
but the snapshots need to be updated! (since its expecting the old
value in the *-test.js.snap file)

I'm not sure if this could be remedied with just changing the logic of the
tests, or if updating the snapshots is a must. 🤔

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/integrations/slack/issues/575#issuecomment-419564695,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHXHLjCENvBPUnhs1ny8omLV7pOVmdGRks5uYt6hgaJpZM4T9kLC
.