[MOB-2606] persistent read state for inbox in-apps #323
Conversation
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
==========================================
+ Coverage 70.35% 70.56% +0.20%
==========================================
Files 60 60
Lines 3701 3734 +33
Branches 430 437 +7
==========================================
+ Hits 2604 2635 +31
- Misses 832 833 +1
- Partials 265 266 +1
Continue to review full report at Codecov.
|
iterableapi/src/main/java/com/iterable/iterableapi/IterableInAppManager.java
Outdated
Show resolved
Hide resolved
| changed = true; | ||
| } | ||
|
|
||
| if (storage.getMessage(message.getMessageId()) != null) { |
There was a problem hiding this comment.
we wont basically need the if condition at this point right? null check is already done and if null, there is a message added in storage above on line 315?
There was a problem hiding this comment.
I did it like this because it's a discretely separate check (AKA should we overwrite), that happens through the null check.
There was a problem hiding this comment.
If the reason is for readability, I think the check should be stored in a named variable to express intent. It takes a while to understand what storage.getMessage(message.getMessageId()) == null and storage.getMessage(message.getMessageId()) != null mean.
There was a problem hiding this comment.
Will add, that's a good idea.
iterableapi/src/main/java/com/iterable/iterableapi/IterableInAppManager.java
Show resolved
Hide resolved
| remoteQueueMap.put(message.getMessageId(), message); | ||
| if (storage.getMessage(message.getMessageId()) == null) { | ||
|
|
||
| if (!isStoringInApp(message)) { |
There was a problem hiding this comment.
I think you don't need a separate method. Just create a local variable like this:
boolean isExistingInApp = storage.getMessage(message.getMessageId()) != null
There was a problem hiding this comment.
If a boolean is only used within a single method, I think it's easier to define it inline rather than declaring a separate method.
iterableapi/src/test/java/com/iterable/iterableapi/IterableInAppManagerTest.java
Show resolved
Hide resolved
vbabenkoru
left a comment
vbabenkoru
left a comment
There was a problem hiding this comment.
Approved with one minor comment.
Ayyanchira
force-pushed
the
MOB-2606-read-state
branch
from
18fdc1d to
23fe3f8
Compare
[MOB-2606] persistent read state for inbox in-apps # Conflicts: # iterableapi/src/main/java/com/iterable/iterableapi/IterableInAppManager.java
🔹 Jira Ticket(s) if any
✏️ Description