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

[EPIC] Feat: /settings + settings.json #363

Open
nelsonic opened this issue Apr 25, 2023 · 4 comments
Open

[EPIC] Feat: /settings + settings.json #363

nelsonic opened this issue Apr 25, 2023 · 4 comments
Assignees
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! priority-1 Highest priority issue. This is costing us money every minute that passes. technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@nelsonic
Copy link
Member

nelsonic commented Apr 25, 2023

To facilitate "Progressive Interface / Experience" [ "PIE" 🥧😋 ] dwyl/product-roadmap#43
Each person using the App needs to have a settings.json
If we were using a Document DB like Mongo, Couch, etc. we would just store a doc in the format: {person_id}.json
But we're using Postgres (for better or worse; IMO better!) so we need to think about how we're doing this ...

We want to have a single endpoint/controller that returns the settings "page" if queried with header HTML
and returns the settings.json if requested with accepts=JSON or the .json suffix.
This is detailed in https://github.com/dwyl/content

There are at least 3 distinct ways to store settings data.

  1. Use one column/field for each setting - i.e. think about the data type up-front.
  2. Use it as a key:value store - this is more versatile but less efficient as everything is a String
  3. Use JSONB to store an arbitrary JSON blob.

What is our preference...? 🤷‍♂️

1. One Column per setting, One Row per person

In this approach we store each setting as its' own column in the DB:

image

The obvious advantage to this approach is efficiency of storage and immediately obvious which settings have been defined for a given person. The only disadvantage I can see is that the table will get very big very fast (number of columns) and may become difficult to read in our Postgres GUI. If we can live with this, I'd go with this approach.

Max columns per table in Postgres is 1600: https://www.postgresql.org/docs/current/limits.html
Doubt we'd ever have that many settings.

2. Row-level History with a Key:Value Store?

In the one row per setting key:value pair scenario

image

Note: I've made the section column a String in this sample table.
But in our actual App this would be normalised to a small int.
And the Map of section Int: Label will be stored in code:

defmodule SettingsSection do
  @moduledoc """
  Define the section struct with desc, id and label keys required
  """
  @type t :: %Section{id: integer(), desc: String.t(), label: String.t()}
  @enforce_keys [:id, :desc, :label]

  defstruct [:id, :desc, :label]
end

Thus the List of Structs would be:

[
      %Section{
        id: 1,
        desc: "Any setting or preference personalisation related to the appearance of the App.",
        label: "Appearance"
      },
      %Section{
        id: 2,
        desc: "Your language and locale preferences.",
        label: "Language Preference"
      },
      %Section{
        id: 3,
        desc: "Notifications you will receive if opted-in.",
        label: "Notifications"
      },
     etc.
]

The advantage of this approach is that we have built-in translatable descriptions/labels that are ready to be displayed in the interface.

3. JSONB ?

I was tempted to use JSONB to directly store JSON in Postgres,
see: https://fullstackphoenix.com/tutorials/add-jsonb-field-in-phoenix-and-ecto
But the author gives the following warning:

"One word of caution is to not go overboard and save data
that its easier to have in as regular columns
" ...

while superficially it appears to make a lot of sense
to store data we expect to be represented as JSON as JSONB,
I get the feeling that we will benefit more from having a schema that we can verify against.
and store using papertrail for full history.

The advantages of JSONB over the other approaches to data storage listed above are obvious:
there is almost no code to write to "transform" the data we just read it from the DB and send it to the client as-is.
To me the biggest disadvantages of JSONB are:

  1. far less efficient storage and potentially slower at scale.
  2. storing the history of settings changes requires saving the entire blob of JSON each time.
  3. more difficult to aggregate the data when there are many records.

Note: sorry if this sounds "incoherent" ... I was hoping it would be insightful into the fact that I think there are alternative ways of achieving the same goal. From the front-end's perspective this will just be a JSON blob that needs to be visualised.

@SimonLab, @LuchoTurtle & @iteles it would be good to get your thoughts on this. 🙏 💬

@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-1 Highest priority issue. This is costing us money every minute that passes. discuss Share your constructive thoughts on how to make progress with this issue labels Apr 25, 2023
@LuchoTurtle
Copy link
Member

I think the third option seems to be the least plausible out of the three. The only advantage is ease of development but you'd still need to parse the settings from it. Additionally, I think we still do need to transform the data. If not from the client but to the client. If we're using a statically-typed language (Dart) on mobile clients, it should expect specific fields and not parse them on runtime.

Regardless, I'm torn between 1 and 2. However, I think the first one is the best option. Although the versatility that having a key-value store provides, it only makes sense when we expect to change the settings several times, which will most likely not be the case. I don't think the reduced efficiency outweighs the versatility factor.

Not to say that I don't have a few constraints with the first option. Every time we want to add a customizable setting, we are forced to add a column and create a migration. In some cases, this can be cumbersome but for this use case, I think it's trivial. And with the added benefit of efficient querying, I believe the first option is the best one out of the three.

@nelsonic
Copy link
Member Author

@LuchoTurtle thank you for this insightful comment. 👌
I hadn't thought about the migrations aspect. 💭

Right now we only have a handful of migrations in the MVP:

image

The closest example is priv/repo/migrations/20220930212128_add_color_to_tag.exs which is just one field:

defmodule App.Repo.Migrations.AddColorToTag do
  use Ecto.Migration

  def change do
    alter table(:tags) do
      add(:color, :text, default: "#FCA5A5")
    end
  end
end

If we end up having a similar order of magnitude of settings as GitHub: https://github.com/settings

image

We could rapidly end up with many migrations as the columns/features would definitely not be added all at once. ⏳

For example Notifications: https://github.com/settings/notifications
image

I can imagine having considerably more settings than GitHub has.
To make Notifications useful to people they need far more fine-grained control over what they see and when.

But to your point: if we are optimising for query speed so the person using the App never has to wait,
then the first option: each setting has it's own column/field is the only option. 💭

@nelsonic nelsonic moved this to ⏳Awaiting Review in dwyl app kanban May 16, 2023
@nelsonic
Copy link
Member Author

@SimonLab would be good to get your thoughts on this if you have a few mins. 🙏

@nelsonic nelsonic assigned nelsonic and unassigned SimonLab Sep 17, 2023
@nelsonic nelsonic moved this from ⏳Awaiting Review to 🏗 In progress in dwyl app kanban Sep 17, 2023
@nelsonic
Copy link
Member Author

@nelsonic nelsonic moved this from 🏗 In progress to 🔖 Ready for Development in dwyl app kanban Oct 3, 2023
nelsonic added a commit to dwyl/book that referenced this issue Oct 3, 2023
@github-project-automation github-project-automation bot moved this to More ToDo ThanCanEver Be Done in Nelson's List Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! priority-1 Highest priority issue. This is costing us money every minute that passes. technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: More ToDo ThanCanEver Be Done
Status: 🔖 Ready for Development
Development

No branches or pull requests

3 participants