fix isSameDay function #453
Conversation
src/utils.js
Outdated
| let currentCreatedAt = moment(currentMessage.createdAt); | ||
| let diffCreatedAt = moment(diffMessage.createdAt); | ||
|
|
||
| if (!currentCreatedAt.isValid() || !diffCreatedAt.isValid()) { |
There was a problem hiding this comment.
Why have both of these checks been removed?
There was a problem hiding this comment.
I found that isValid returns true for all the cases that this test may be trying to avoid. For e.g. moment(undefined).isValid(), moment(0).isValid(), moment({}).isValid() will all return true. I guess one case where it will return false is for moment(null).isValid(). I will put the check back in.
src/utils.js
Outdated
| @@ -4,13 +4,13 @@ const DEPRECATION_MESSAGE = 'isSameUser and isSameDay should be imported from th | |||
|
|
|||
| export function isSameDay(currentMessage = {}, diffMessage = {}) { | |||
|
|
|||
| if(!diffMessage.createdAt) { | |||
|
@cooperka I have made the changes you requested. I am not sure how I can help with the testing. I have tested it on my end but I guess it would be good to know that this change is not breaking anything expected for some other users. |
|
@cooperka Hey, You are absolutely right, we must have some tests here.. I promised myself I'd find time for it but couldn't get to it :( I think the hardest part is to set it up, from there on we could all just add tests as we add/change functionality. If anyone has some time to set up some basic tests it would be amazing, otherwise I'll get to it at some point :) |
isSameDaymethod was returningtrueif we feed acurrentMessagewhich is created today and an emptydiffMessage. That is becausemoment(undefined)call will initialize the moment object to today's date. I think we were usingisValidto make sure that there is a validcreatedAtforcurrentMessageanddiffMessagebutmoment(undefined).isValid()will return true.I have changed the method to make sure that we have a valid
createdAtdate fordiffMessageand remove theisValid()calls as they will always return true.