Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Preferences KDD #5729
base: develop
Are you sure you want to change the base?
Preferences KDD #5729
Changes from 1 commit
35b1fca
c6cea4b
e67a178
60c807e
c2ac5a6
be7e1ae
4676cac
a5f91a7
ffb0a04
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm not entirely sure if I still agree with this comment, given it's one struct we don't have different
prefs
there's just one set of prefs for the whole system (in my understanding?)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.
I'm not sure what you mean by having different prefs, but having a set (of different?) prefs? If it helps clarify there was a bit of implementation discussion that came up - basically I thought we were heading towards a bigger json pref that was all of a store's prefs, but was worried that we'd be pinned for potential sync situations, leading to clarifying another thing...
In OG store prefs and global prefs are both controlled on the central server, we're not worried about merging pref records through sync atm. That's likely to continue to be the case in OMS, though it'd be nice to keep the door open for some store prefs to be controlled on the remote site. At any rate, if prefs are being edited on both remote and central for a given store, individual records for prefs makes conflict resolution easier. Unless we use CRDTs 😁.
Anyway, so assuming that there are lots of individual prefs, to initialise the BIG STRUCT we'd do something like query all the prefs for a given store_ID then have fns on the struct for getting a particular pref from that selection or default.
Does that make sense, or does that raise more questions? In fairness if we're all on the same page then this con can be removed because the convention is taking shape already!
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.
Does my rust code below help clarify the
BIG STRUCT
approach?I was thinking for the first implementation at least, we'd have a single database row per store, and a single global row. The internal JSON structure would be the same, although we wouldn't need to serialise the
None
fields.If we want to allow remote sites to edit their own store_prefs, then I think we go with the last update wins initially.
Could alway extend to allow some kind of CRDT for merging, or split store_prefs into
remote_store_prefs
andcentral_store_prefs
as a simpler version of broken up prefs.I'm personally, trying to avoid having lots of different prefs in the database to need more complex querying, mental overhead to decide is this a new pref row, is it part of another pref, etc. To me the simplicity of the big struct is that everything is there in one place. It's easy to follow through the flow from db records to what gets passed to in the UI.
I might be on a different page to others though so wanted to share that rust example to explain.
I agreed there might be limitations and challenges that come up with this approach, but I think it's a good way to start and only with good reason e.g requirements & use case do we extend it further.
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.