Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(indexeddb): Added dexie to support indexeddb storage AP-366 #814

Merged
merged 18 commits into from
Jan 12, 2022
Merged

feat(indexeddb): Added dexie to support indexeddb storage AP-366 #814

merged 18 commits into from
Jan 12, 2022

Conversation

josephmcg
Copy link
Contributor

@josephmcg josephmcg commented Dec 28, 2021 •
edited
Loading

What this PR does 📖

  • uses dexie for better indexeddb api. I researched other options (idb,localforage) too, but dexie seems like the best
  • on message load - if nothing is stored in indexeddb, fetch every message from textile and set in indexeddb
  • on message load - if indexeddb has info, pull any new messages from textile based on last inbound message timestamp. combine indexeddb messages with new textile messages and set in store.
  • on message send/edit - set in indexeddb. Edit replaces message in indexeddb similar to how textile would return an edited message

Which issue(s) this PR fixes 🔨
#AP-366

Special notes for reviewers 🗒️

  • indexeddb data is found in the chrome dev tools Application tab
  • since textile stores message times in unix nanoseconds, the textile fetch will return the latest message, even if nothing is new. We may want to pull the lastInbound value directly from textile rather than set it ourselves using Date.now()

Additional comments 🎤

@github-actions github-actions bot added the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Dec 28, 2021
@josephmcg josephmcg changed the title feat(indexeddb): Added dexie to support indexeddb storage AP-366 wip WIP feat(indexeddb): Added dexie to support indexeddb storage AP-366 Dec 28, 2021
@stavares843
Copy link
Member

@josephmcg is this still a WIP or its ready? 🔨

@josephmcg
Copy link
Contributor Author

@stavares843 WIP - I'm thinking of adding some additional functionality to it first. Right now, it's just storing conversation info every time its fetched

@josephmcg josephmcg added the draft A developer wants eyes on this PR, but they don't think it's ready to merge. label Dec 29, 2021
@stavares843
Copy link
Member

@josephmcg just checking, is this ready to review/test? 🔨

@josephmcg
Copy link
Contributor Author

not yet, I'm taking a couple vacation days. I'm hoping to finish it up tomorrow

@stavares843
Copy link
Member

neat, thanks 🎉

@github-actions github-actions bot added the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Jan 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This Pull Request may conflict if the Pull Requests below are merged first.

#846
conflictable files: locales

@josephmcg josephmcg removed the draft A developer wants eyes on this PR, but they don't think it's ready to merge. label Jan 5, 2022
@josephmcg josephmcg changed the title WIP feat(indexeddb): Added dexie to support indexeddb storage AP-366 feat(indexeddb): Added dexie to support indexeddb storage AP-366 Jan 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This Pull Request may conflict if the Pull Requests below are merged first.

#872
conflictable files: locales
#832
conflictable files: locales
#846
conflictable files: locales
#840
conflictable files: locales
#681
conflictable files: locales,package.json

This was referenced Jan 5, 2022
Closed
Merged
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This Pull Request may conflict if the Pull Requests below are merged first.

#832
conflictable files: locales

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This Pull Request may conflict if the Pull Requests below are merged first.

#840
conflictable files: locales

@github-actions github-actions bot added missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa and removed missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa labels Jan 5, 2022
iltumio
iltumio requested changes Jan 5, 2022
View reviewed changes
Copy link
Collaborator

@iltumio iltumio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the most important part is to move the indexedDb async logic from the mutation to an action or into a store plugin. See comments

store/textile/actions.ts Outdated Show resolved Hide resolved
plugins/thirdparty/dexie.ts Outdated Show resolved Hide resolved
store/textile/mutations.ts Outdated Show resolved Hide resolved
@josephmcg josephmcg added Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. and removed Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. labels Jan 6, 2022
@stavares843 stavares843 requested a review from iltumio January 7, 2022 18:09
@github-actions github-actions bot added the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Jan 8, 2022
@github-actions github-actions bot added missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa and removed missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa labels Jan 11, 2022
WanderingHogan
WanderingHogan approved these changes Jan 11, 2022
View reviewed changes
Copy link
Contributor

@WanderingHogan WanderingHogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is neat

@josephmcg @WanderingHogan
added minor dexie config
f55eead
@josephmcg @WanderingHogan
working state for dexie convo storage
8593231
@josephmcg @WanderingHogan
better dexie organization - working status
c1c23c3
@josephmcg @WanderingHogan
fix: refactor and delete unused file
0c308b2
@josephmcg @WanderingHogan
store messages rather than conversations
8540289
@josephmcg @WanderingHogan
cleanup old code
64cf2ae
@josephmcg @WanderingHogan
only write to idb if out of date
bcce44b
@josephmcg @WanderingHogan
fetched last message timestamp if data is stored locally and passed t…
ff972e2 
…o MailboxManager
@josephmcg @WanderingHogan
fix(dexie): fix edit message indexeddb storage
034cddd
@josephmcg @WanderingHogan
fix and refactor db storage
bc65529
@josephmcg @WanderingHogan
rework data fetch and add support for partial fetch
b08a49f
@josephmcg @WanderingHogan
feat(indexeddb): integrate indexeddb with textile messages
a83097d 
* if nothing stored, fetch every message and store
* if stored, combine indexed messages with new messages from textile
@josephmcg @WanderingHogan
fix(message timestamp): convert inside mailbox manager instead
7e6dda3
@josephmcg @WanderingHogan
fix(indexeddb): restructure into actions rather than mutations
9feaba2
@github-actions github-actions bot removed the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Jan 11, 2022
@josephmcg
Copy link
Contributor Author

josephmcg commented Jan 12, 2022 •
edited
Loading

Manuel gave me some good feedback regarding the data structure. I'm going to finish refactoring within a day or two

edit-- had some time this morning. ready for Testing/QA

@iltumio iltumio merged commit 394b6a3 into dev Jan 12, 2022
@iltumio iltumio deleted the dexie branch January 12, 2022 14:48
@github-actions github-actions bot removed the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iltumio iltumio iltumio approved these changes

@Jekrimo Jekrimo Jekrimo approved these changes

@WanderingHogan WanderingHogan WanderingHogan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@josephmcg @stavares843 @WanderingHogan @Jekrimo @iltumio