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.

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Preferences KDD #5729

wants to merge 4 commits into from

Conversation

Chris-Petty
Copy link
Contributor

Fixes #5683

@github-actions github-actions bot added this to the v2.5.0 milestone Dec 10, 2024
@github-actions github-actions bot added KDD Build Tested: None This issue cannot be build tested or will not be build tested Team Piwakawaka James, Carl, John, Zachariah labels Dec 10, 2024
@Chris-Petty
Copy link
Contributor Author

Anything to add, go for it!

Anything I should elaborate on?

I decided for first draft just to get the contextual stuff down and a crack at requirements. Put in the OG prefs option. I haven't gone much deep into implementation details, figured to just agree on requirements and preferred schema/data structure first.

@jmbrunskill
Copy link
Contributor

A thought to add, can we export a group of gloabl and/or default store prefs to apply to a new site/demo?
E.g. if we have a new feature that's relying on a set of prefs, it would be good if we can provide that to support & QA to import for testing or setup.

Copy link
Contributor

@jmbrunskill jmbrunskill left a comment

Choose a reason for hiding this comment

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

Thanks @Chris-Petty I'm suggesting a third option, which is kind of like Option 1 but a bit more limited/focussed. Mostly thinking about how we can use this to create a pref editor UI, think the approach I suggest would work.

More complex stuff wouldn't be done with preferences, or we'd need to extend teh capabilities of the pref options/types.

decisions/2024-12-03_preferences.md Show resolved Hide resolved

_Cons:_

- Some preferences might need more than one FK, e.g. user pref for a specific store or machine.
Copy link
Contributor

@jmbrunskill jmbrunskill Jan 6, 2025

Choose a reason for hiding this comment

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

Suggested change
- Some preferences might need more than one FK, e.g. user pref for a specific store or machine.
- Some preferences might need more than one FK, e.g. user pref for a specific store or machine.
### Option 3 - Similar to `properties` and `name_properties` structure
With this option we'd define a list of `prefs` in the database that could be assigned, and what possible values they have e.g. float, int, boolean, controlled list.
Each preference has a `unique_key` assigned, this key is used to assign the value to when serialising/deserialising to json.
Each pref would record if it's relevent to apply at a global, store, or other level.
In sync we'd sync a `preference_data` record for each store and one global (for now) could add user, or machine etc as needed in future. This would have a json payload similar to Option 1.
There's nothing to stop us still deserialise them in a rust struct as needed
This option would allow us to develop a preference editor UI similar to what we have with editing store/name properties.
If you need some more complex type in your sync record, then it probably is worth creating a dedicated ui and management for this struct.
_Pros:_
- Re-uses an existing pattern
- Clear path for creating a UI without lots of over head for adding a new pref
- Can still deserialise to specific structs if needed
- Could be used by plugins to add additional prefs as needed
_Cons:_
- Could be larger payload as prefs aren't broken down into different areas
- Need to insert prefs into database rather than just adding to code? Maybe there's a code first way to achieve the same thing? Enums?

image

@jmbrunskill jmbrunskill self-requested a review January 6, 2025 03:42
@roxy-dao roxy-dao modified the milestones: v2.5.0, V2.5.0-RC1 Jan 8, 2025
Co-authored-by: James Brunskill <james@sussol.net>
Comment on lines 133 to 143
_Pros:_

- Re-uses an existing pattern
- Clear path for creating a UI without lots of over head for adding a new pref
- Can still deserialise to specific structs if needed
- Could be used by plugins to add additional prefs as needed

_Cons:_
- Could be larger payload as prefs aren't broken down into different areas
- Need to insert prefs into database rather than just adding to code? Maybe there's a code first way to achieve the same thing? Enums?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion @jmbrunskill !

Would work quite well for plugins as they'd just be centrally controlled records that sync everywhere.

I'd be slightly less concerned about the larger payload. We have already the "custom_data" field and that hasn't blown up too badly despite the years. In some ways it's quicker to sync 1 big record over 1000 pref records, far few queries. Tricky if someone puts something chunky in it though, such as a base64 encoded file. e.g. a plugin for adding an album of photos of a store, but rather than using document sync they just encode it in the plugin config 😆.

Speaking of plug config, I had been thinking that if plugins had config then the plugin should include the UI for the configuration that the FE loads up. That might have some flexibility advantages, but the way you suggest would probably cover most cases and be easier than cooking a UI for every plugin.

I'll add to the other options details on what config options they have.

Co-authored-by: James Brunskill <james@sussol.net>
@Chris-Petty
Copy link
Contributor Author

Chris-Petty commented Jan 20, 2025

@jmbrunskill what your suggestion quite reminds me of vscode extension configurations which got me sniffing around how vscode does it. Here is rust-analyser's config definition for example.

Here is the docs for extension configuration https://code.visualstudio.com/api/references/contribution-points#Configuration-property-schema. One thing to note is that it is a superset of JSON schema, which we have worked with on presenting with JSON forms previously.

All this kinda shows to me that your approach could easily handle things such as drop down options and some degree of grouping. Surely if the approach is flexible enough for vscode extensions and generating the settings UI from the JSON schema surely it'll be good enough for our configurations/preferences 🙂.

I am wondering if I'm diverging from option 3 though - or at least I'm guessing on how we manage the configuration data. Some amount of base preference_config for preferences of standard functionality, and separate records for each plugin. Haven't thought out exactly how it's queried for.

n.b. I'm worried about calling the config record just preference cause it'll 100% create lots of confusion with those versed in OG or outside of dev (because from a UI perspective we talk of preference being the thing you turn on/off)

Each preference has a unique_key assigned, this key is used to assign the value to when serialising/deserialising to json.

Just confirming how devs might use this - logic using a preference should be expected to use the unique key right? some scenarios to convey what I mean (and tbh wouldn't be any different to the first 2 options! "unique_key" is just the pref.item of OG):

  • In backend, code can query the preference_data table based on a static &str (constant?)
  • For frontend usage:
    • gql layer, as the backend, can include relevant preferences in the gql schema, the backend would include them via as above
    • We could have an gql endpoint dedicated to fetching a pref data, to be used arbitrarily by frontend logic (I sorta like this less than the above option)
    • Must eventually have an endpoint for mutating a pref data

Perhaps this is relevant and plausible for all the options, which indicates I may be in the weeds 😉.

@roxy-dao roxy-dao modified the milestones: V2.5.0-RC7, v2.6.0 Jan 22, 2025
@mark-prins
Copy link
Collaborator

Looking good @Chris-Petty - nice and comprehensive!
I was also wondering about the vscode approach, I like the hierarchical nature of it. that's achievable with any of your options though, with a differing level of effort.

Agree that having a pref editor is nice - and I like the approach of having a single table with fields for the association of a pref to a user/store etc.

which is to say that I like option 1 and then preferred Jame's third way when that popped up. happy with any of these approaches tho.

I'm relaxed about migrating from an existing OG implementation, though ideally we'd accomodate that somehow

@CarlosNZ
Copy link
Contributor

CarlosNZ commented Feb 4, 2025

I basically like Option 3, but I'd like to offer what I think are important "requirements" and try and work towards a solution that achieves all if possible -- I think Option 3 is heading in the right direction, but would be good to see if we're all on the same page about the importance of the following:

Requirements (see if you agree):

  • Invisible to front-end: app should just receive a single structure of all prefs and not care about the source (user pref/store pref, etc)
  • Applied heirarchically
    • I.e. can be defined for store, user, or globally, with configurable rules about what the precedence is. e.g. some prefs might want the "store" setting to overrule any "user" setting, but others might need to be the other way round
    • Should aslo handle "partial" definitions, i.e. a user doesn't have to have a setting for each pref, it would just fall back to store, etc, until ultimately hitting a global, hard-coded default
  • Minimise workload of keeping configurations UI up-to-date/in sync with definitions (Hello JSON Forms!)
  • Capacity to handle plugin preferences
  • Easily extensible to different preference structures without needing to create "special" tables for complex cases in the future
  • Very broad interpretation of "preferences", i.e. anything "configurable" should be able to captured under this same umbrella structure, so it's all one main concept
  • Types easily generated for front-end for any type of preference structure

To this end, I think it's useful to distinguish between 3 different "concepts" that each require their own implementation (although obviously inter-related), would be good if we could agree on decent terminology for each of the following:

  1. Preference type definitions -- the name/code and data structure/type for each possible preference (could be as simple as just "boolean" up to complex nested objects)
  2. Preference "structure": i.e. what preferences (from 1) can be applied at what level (global/store/user), and the priority/fallback order. I'm thinking this is globally configurable, but we may want to impose hard restrictions for some (e.g. some prefs might have to be only global to make sense)
  3. The actual preferences -- i.e. the values set for each user/store/system

I have written up a more detailed design/schema that can hopefully accomodate all the above, but I won't post that here yet as it still needs a bit more thought and it'd be good to see if it's even required based on further discussion, but the key points are:

  • Loosely akin to Option 3
  • Rust types as primary source of truth for each pref definition (not database schema -- database just gets JSON)
  • Auto-generated JSON schemas based on types
  • JSON Forms UI for configuration based on those schemas
  • Things like "grouping" and other visual relationships between prefs can be defined in UI Json Schema, and don't need to be considered in the underlying schema
  • Plugins can provide their own defintions for both the preference "type definitions" and "structure" (1 & 2 above -- probably need better naming to distinguish those), and the same core "engine" would generate schemas and UI.

@Chris-Petty
Copy link
Contributor Author

I'm relaxed about migrating from an existing OG implementation, though ideally we'd accomodate that somehow

I'm not sure what exactly you mean by accommodate the OG implementation? I guess I can see some teething pains:

Old OMS sites will still rely on the old OG central server for existing prefs. So we'll have to configure anything twice, not sure if there is any mechanism that can address that without feeling very... spaghetti. e.g. the OMS central server could map the new structure to old structures and sync that to OG central...

Old OMS sites will also have to smoothly migrate the old pref data structures to the new, that shouldn't be too hard in any of the solutions IMO.


One note for option 3 is there is possibly some overhead, or at least it's moved the overhead. In Option 1 many prefs will just be essentially:

SELECT data FROM pref WHERE item = 'some pref' AND store_id = 'some id';

->

{ "value": true }

Complex data structures in option 3 might just need a bit more parsing work. That's probably negligible though :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Tested: None This issue cannot be build tested or will not be build tested KDD Team Piwakawaka James, Carl, John, Zachariah
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KDD of preferences structure
5 participants