-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Return early if no valid sequenceNumber can be found #2777
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.
Looks good!
// update the last read. Most likely, we have just created the report and it has no comments. But we should err on | ||
// the side of caution and do nothing in this case. | ||
if (_.isUndefined(sequenceNumber) | ||
&& (!reportMaxSequenceNumbers[reportID] && reportMaxSequenceNumbers[reportID] !== 0)) { |
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.
NAB, but why not use ??
(reportMaxSequenceNumbers[reportID] ?? 0) !== 0
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.
Why should we use it? 😄
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.
Here's the result I want...
Only reject the value if undefined
or null
but not 0
and figure that out by looking for something falsy
!reportMaxSequenceNumbers[reportID]
and (&&
)
that is not 0
reportMaxSequenceNumbers[reportID] !== 0
since if reportMaxSequenceNumbers[reportID]
was 0
it would be falsy.
🚀 Deployed to staging in version: 1.0.41-8🚀
|
🚀 Deployed to production in version: 1.0.44-0🚀
|
cc @roryabraham
Details
Updates a check that only checks for an "undefined" sequenceNumber when it should be ignoring any invalid values for a
sequenceNumber
. Seems like this PR actually did not make it to production. But I also happened to spot anull
value inreportMaxSequenceNumbers
on dev so I think this is a good change regardless.Fixed Issues
No issue, but related to #2704
Tests
QA Steps
Same as #2704
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android