Merge predefined* and user* tables

[JulianDietzel]

The user tricks and the predefined tricks, just as the user combos and the predefined combos are stored in separate tables in the database. Yet, it is made sure in almost every function that adds to those tables, that there are no duplicate ids between the tables. Also, when tricks or combos are fetched from the database, the predefined and the user tables are almost always if not always merged. This makes the code harder to read, makes functions longer (as always both tables need to be queried and the results need to be merged) and creates the need for functions like the mergeLists one.

Couldn't the tables just be merged into one, with an additional column isPredefined, which just stores a boolean value and specifies whether the trick is a predefined one or not. I'd estimate that this would shorten the db.js file by about 20% and increase readability a lot. Also it would prevent the tables from getting out of sync, if someone were to accidentally only change / update one of the tables but not the other.

I'd be interested if there is perhaps a good reason for the current architecture that I am missing. If not, I'd suggest merging both the predefinedTricks and userTricks tables and the predefinedCombos and userCombos tables.

[bastislack]

@weberax was talking about a database refactoring. So maybe this is something we can discuss here?

[weberax]

I like the idea!

The original idea of having two different tables, was

1. to have the predefinedTricks seperate to update them easily, without tricks getting mixed up (and of course keeping the user data)
2. and always keep the orginal details of a trick, so that when a user changes attributes of this trick, he can reset them, by just deleting the entry in userTricks.

But since we decided to have the ids in the google form fix, 1. should not be a problem anymore.
And 2. should also not be a problem.

So to summarize what needs to be done (analogous with the combos):

• combine the two tables, overwriting attributes of the predefinedTricks table with the one from the userTricks

• adapt all the functions

• add a function to reset one or all tricks back to the default values

• modify the populateTricks function so, that when a newer predefinedTricksVersion is released only the values, which where not modified by the user are updated (suggest in Updates to the tricklist are not applied if a trick is saved in the userTricks #194)
  maybe this could be done by only allowing the user only to change certain attributes, like

  • level
  • alias
  • (what else?)

  which will get saved as "userLevel", "userAlias"... and will overwrite the other values.
  And for Tips i suggest to do something like, merging the predefinded tips with the "userTips".

This might also fix #240

[WaldemarLehner]

Co-Authored by @JulianDietzel

The Document defines changes to the Database Structure that we deem useful.
The first section will be an Outline of the proposed changes. The following sections will talk about the details, the reasons behind decisions, considered alternatives, etc.

Outline

predefinedCombos and userCombos get merged into combos .
predefinedTricks and userTricks get merged into tricks .
versions gets dropped and replaced with a hashing mechanism.
A new metadata-Table gets added. It defines user-defined metadata for both predefined and user-defined tricks and combos.

The following text will talk about Tricks, but note that all mentioned paradigms apply to Combos, too.

The user will no longer be able to modify predefined Tricks. They can however use predefined Tricks as a template.

The Primary Keys get turned into composite keys.
Instead of indexing an Entry by the id, two values ([id, trickType]) are used for indexing. TrickType is an Enum stored as a string and validated with zod during insertion. This means that there can be multiple tricks with the same ID as we can have one Trick identified by [1, UserTrick] and another by [1, PredefinedTrick]. This way we can eliminate ID collisions between Predefined and User Tricks.

Below you can see the new proposed Database Structure.

Notes

1. This might need a better name. We were also thinking about TrickVariant, but this will get confusing down the line in case "Trick Variations" get added.
2. This structure only works if Metadata is identical between Combos and Tricks. Can we assume this? Or are there any values that are specific to Combos or Tricks?
3. This note is regarding a new Trick- and ComboType: Orphan. An "Orphan" defines a predefined trick where the user has added any kind of Metadata where the trick has been removed from the upstream data source (currently Google Sheets). This is done with the assumption that if a user has made an interaction with said Trick or Combo that they might still want to interact with it and keep their progression.

Assumptions / Requirements

In order to create a purposeful architecture we made some assumptions on what the application should / wants to be.

These assumptions are at the core of how the app should function and how it should be designed.

We made the assumption that the primary tasks of this app are to

1. allow the user to manage their own tricks and combos
2. allow the user to track their progression for tricks and combos.
3. serve as a database of "official" tricks and combos, with their respective difficulties (potentionally to be used for competitions).
   → "official" tricks do not have to be editable by the user. They should stay in sync between all users.

If you feel like our assumptions are wrong or incomplete, please correct them. It is critical that we get them right.

Updading Predefined Data

Currently, a version number is used for updating predefined data. If the version number provided by a JS Object is bigger than the version in the Table, an update takes place.
With our proposal we recommend the use of a hash-Function. This function will, for a given input, always return the same hash — with the idea that any change in the input will result in a completely different hash.
This way, if the Input (predefined Trick JS Object) ever changes, evaluating them would result in a different hash.

We propose to store the Hash in a Dexie Table as a simple string. On startup, the predefined data gets run through the hash function. In case there is a difference between stored and computed hash all predefined rows are deleted from the Database and updated.

The following figure shows the steps taken during startup to check if the predefined tricks need to be updated, as well as the general procedure taken if an update is needed.

sequenceDiagram
    participant A as Start
    participant B as Version Table (Dexie)
    participant C as Trick Table (Dexie)

    A->>A: Load JS Object with <br>predefined tricks
    A->>A: Hash the object <br>(computed hash)
    A->>+B: Check if a hash is <br>already stored (stored hash)
    B-->>-A: 
    alt stored hash == computed hash
        note over A,C:  Stop. Already synced
    else else 
        A->>+C: Update predefined Trick Entries in DB<br>(see other Diagram for further info.)
        C-->>-A: 

        A->>B: Write Hash
    end 
    

Loading

Handling Deleted Predefined Data

As mentioned above, a new Type of Combo and Trick, the "Orphan" gets added.
If a Predefined Trick or Combo gets deleted from the "source of truth" (the JS Object exported by the virtual module), it gets turned into an Orphan.

Here are the strategies we identified for dealing with deleted predefined tricks:

1. Orphan Tricks as 3rd Trick type

Predefined tricks that get deleted are turned into orphans. They then function the same way as user tricks, but are marked as Orphans in the UI.

2. Orphans as intermediary (in the limbo :D)

Predefined tricks that get deleted are turned into orphans. While in that state, they are excluded from any lists and searches. The user is prompted to either convert the tricks into User Tricks or to outright delete them.

3. Simply Delete Trick like it never existed (without Orphan Type)

This is probably undesirable because users could lose their progress or be confused as to why some tricks suddenly disappeared.

4. Autoconvert to User Trick (without Orphan Type)

This would skip the orphan stage and simply convert any to-be-deleted predefined trick into a user trick.
This is a very simple solution with the downside that the user might be confused as to why there is a new trick in the list of tricks that were explicitly defined by them.

Waldemar prefers Option 2. Julian prefers Option 1 and 4. All of them would work. We would love your input on what you deem best.

Here you can see an overview of the deletion process for all 4 options.

How we came to this design

There were some other approaches considered, with the most promising being the use of JS-Objects that contain the predefined tricks.
The Database-Instance would access that object instead of the actual indexed DB if it wanted to query predefined data.
The big benefit of going that route is that "constant" predefined data is outside the database. We would not need to care about updating predefined entries in the Database.
The big downside however is that we cannot query predefined tricks using a Dexie query. Another downside is also that we would also need extra logic to normalize access between two separate data sources with two separate access strategies.

Rationale

This design allows us to keep one "source of truth" — that's the JS Objects defining predefined tricks created based on the Google Sheets tables. By having everything in the Dexie DB we can make use of Dexie's querying functionality without needing to join data from different sources (e.g. separate userTrick and predefinedTrick tables).

In order to satisfy the requirement of serving as an "official" database of tricks, removing the functionality for users to edit predefined tricks allows for a consistent global state of predefined tricks. This way all users will always have the same "official" trick information which makes it easier to use for competitions.

In order to still allows users to track their progression and make custom notes on official and user-defined tricks we opted to create a new table, metadata. This table is decoupled from the main trick table to allow for easier updates. It also makes it more clear what is "predefined" and what is "custom" data for predefined tricks from a developer's perspective.

Related issues and open questions

If there are any plans to add any new features which would affect the database structure, now would be a good time address them.

Here are some open issues that come to mind:

• Mark "new" on tricks/combos that have recently been added #197 — We could add a new field createdAt which stores an epoch value for when the trick was added
• How can we handle doubles and triples on tricks where it doesn't really matter? #171 — Not sure what the solution would be yet.
• How can we handle tricks like 'Handpop to Nevermind'? #170 — Adding the Enum Value Various to the Position-Enum as suggested by @bastislack would work. There is however the unresolved question of different difficulties. One might consider adding null as a valid value for the difficulty column.

Another question in case you like the proposed concept of user notes:
Should they consist of a string, or of a list of strings?

[bastislack]

Assumptions / Requirements

You pretty much nailed it!
Of course there will be additional requirements in the future (such as having user accounts and being able to share tricks and stuff like that) but the three requirements that you listed won't change anymore, I guess.

Updating Predefined Data

The process you described for updating tricks sounds good to me.

Handling Deleted Predefined Data

I sat down with @Felix-Hmpl and discussed about this.
Some points we came up with:
Basically, as you already said, approach 3 is not desirable as the user does not want tricks to disappear (probably even if there was no interaction with the trick until now). Approach 2 could be a bit intrusive, if we have a big trick list update and decide to delete 15 tricks the user will be annoyed to have 15 pop-up messages asking, whether to delete the trick or to convert it to a user trick.
What we thought would be a combination of the approaches 1/4 and 2. When a trick is deleted it automatically is converted to a user trick (or orphan trick). When the trick is accessed for the first time after the update, that's when a pop-up is shown asking the user whether the trick should be deleted or converted to a user trick (probably the user should just see the options 'Delete' and 'Keep It'). I am not sure if the orphan type is really needed for this approach. If we can just implement this logic in the user tricks, then I think adding a new trick type is unnecessary code, but if the logic is a lot of code that fits better to the orphan type then we should use it. I am thinking about the single responsibility principle vs. don't repeat yourself principle here: if we don't use orphans the user trick class might get bloated with things that do not belong here. However, if the orphan tricks have a lot of duplicate code from the user tricks we should probably not use it (if inheritance only adds the necessary code then probably that would be the way to go?).

What do you think about the approach?

Some Remarks

A minor comment to the DB design: The optional keyword for technicalName and alias is the other way round (every trick has a technical name and some tricks have aliases).

For now it seems fine to me that the metadata is the same for tricks and combos. If this changes it still shouldn't pose a problem as we could just decide to not show certain fields in the UI for combos or tricks.

Since we are saying that users can't modify predefined tricks (only by using them as a template for a user trick) I don't see the necessity of having a composite key for the tricks and combos. The predefined trick ids start with 10000 and I don't see any user creating 10000 tricks for now (not even Ian :D ). So we could just keep it as it is for now without problems, I guess.

I have created some additional columns in the trick list. One will be used for #197, 'dateAdded'.

Two other columns are one way to deal with #171. We have too many tricks now to show all of them on the main screen, especially tricks that pretty much noone cares about. So I added a bool column 'isImportant' and a column 'isVariationOf'. If a trick is not important at least one trick id has to be filled in the corresponding 'isVariationOf' field, this could be used to create little drop downs on tricks that have unimportant variations and show them if the user clicks on the drop down button.

I have not thought too much about #170 yet but one possibility would be (kind of related to #171) to allow an array of start and end positions to be used, where the first entry in the array is used as the main combination of positions and the other entries show other possible variations. But I think this can be handled in a later release, for now we have more important things to work on.

Let me know what you think and feel free to correct me with things that I got wrong.

[WaldemarLehner]

Haven't talked to Julian about this yet, so here are just my 2ct:

Updating Predefined Data

I see some mentions of inheritance. I don't see how that is relevant. Any logic should be pretty much Trick/Combo-Type agnostic. No inheritance takes place. DRY is not violated.

Your mentioned approach works for me. I would still argue that the orphan Type is helpful here. How would you track of a Trick has been "orphaned"?

Some Remarks

If this changes it still shouldn't pose a problem as we could just decide to not show certain fields in the UI for combos or tricks.

The predefined trick ids start with 10000 and I don't see any user creating 10000 tricks for now (not even Ian :D ).

The Issue here is abandoning a clean data structure. A good database should not rely on "magic numbers" or have some hidden meaning behind values. But maybe I am overdoing the entire engineering part :D

I would like you to take a look at this from the point of view of a new Dev. I would argue it is easier to infer if a DB entry is a User-Entry of a Predefined Entry with the composite key, instead of having a magic number (in this case, 10_000).

I don't see any user creating 10000 tricks for now (not even Ian :D ).
While I agree, I would argue that it is a bad decision to limit the max count like that. The database should not restrict how many tricks a user can create.

So I added a bool column 'isImportant'
Could go into detail on how isImportant would be handled? What is the default value? When does the value change? How does isImportant affect search results?

where the first entry in the array is used as the main combination of positions and the other entries show other possible variations

I don't like this because of the same issues mentioned above (e.g. first entry is implied to be primary)
We could have a startPosition and endPosition, and an option alternativeStartPositions and alternativeEndPositions .
Hmm.. but then also… having two fields for that is also rather ugly. @JulianDietzel, what do you think? :D

I guess it boils down to: Does the start Position need to be queried?
Alternatively… Another Table could be created that defines all possible alternative combinations.

interface Table {
  id: number // PK, autoincrement
  trickId: number // Part of Composite FK; 
  trickType: TrickType // // Part of Composite FK;
  startPosition: Position
  endPosition: Position
  difficultyOverride?: Difficulty // Just a thought :P  
}

so something like

---

id	trickId	trickType	startPos	endPos	difficultyOverride
0	2351	UserTrick	SOFA	KOREAN
1	2351	UserTrick	SOFA	SOFA	4
2	2351	UserTrick	SOFA	BELLY	6
3	2351	UserTrick	SOFA	CHEST	5

would define 4 alternative Start / Stop Positions

Sorry if the thing is a bit incoherent. :P

[JulianDietzel]

Handling Deleted Tricks

I like your proposal of showing the tricks like they are user tricks in the trick list but then prompting the user about how to handle them when the open it.

On the question of whether the Orphan-Type is needed: Firstly I think there is a small bit of confusion around what the Orphan-Type actually is:
@WaldemarLehner and I intended for it to only be an Enum, not a full class. There would be a class called, for example, Trick and this class then has one attribute of type TrickType, which indicates whether the trick is a User-, Predefined- or Orphan-Trick. This way no special logic needs to be (re)written for the Orphan tricks. Adding the option to the TrickType enum would simply offer a way to show that the trick is now neither predefined anymore nor a proper user trick yet.
Therefore this should not violate the DRY or single responsibility principle, form what I can see.

This was the best way to store the information that a trick is in this kind of "limbo", that we could come up with. Repeating Waldemars question:

How would you track of a Trick has been "orphaned"?

Some Remarks

• Good spot with the alias vs technicalName, I guess we can just fix this in the original comment to keep things clean
• On using a composite key instead of splitting the id-space into two sections below and above 10000. I totally agree that the current approach should work under all normal circumstances, so if you insist, we can keep it. But as Waldemar has already pointed out, having the composite key removes this "magic number" and avoids collisions from the start. No way to accidentally insert a user trick with an id greater than 10000 and have it act like a predefined trick. To demonstrate how the system can be more intuitive to use, consider these to queries to find all predefined tricks:

  db.where('trickType').equals('PredefinedTrick')

  vs

  db.where('id').above(10000)

  The first option clearly states what the query is doing, the second would probably need an explanatory comment. With a short query this is still simple enough, longer queries will become easier / harder to read though.
• A dateAdded collumn seems like a good simple solution. We should add this to the original comment.
• Your solution to How can we handle doubles and triples on tricks where it doesn't really matter? #171 seems good to me. Since the isImportant field seems to be always be described by variationOf.length == 0 I think the whole field could be droppend an only the variationOf field be kept. What do you think about this?
• I agree that How can we handle tricks like 'Handpop to Nevermind'? #170 is not the most important issue at the moment, but considering it now is still a good idea so that we can build it into the system instead of patching it onto it later. I haven't thought of the solution all to much but here would be my idea:
  It seems that the issue is mainly concerned with combos. Since it might be hard to put all the fun / weird / personal descriptions of what to do and when into a rigid system I'd propose we just allow the user to optionally attach a note to each of the tricks in a combo. A combo is not a list of tricks anymore but a list of tuples with each a trick and (optionally) a string of text. We then display these notes above the tricks.
  Optionally we could take this yet a step further, and allow the tuples to either contain a trick, a trick and a not or only a note. So if something funky should happen between tricks, the user can just add it as text and it shows up similar to a trick in the UI (maybe have a different border color or something, to differentiate between notes and proper tricks).
  What do you all think of this idea?

[weberax]

Handling Deleted Tricks

I like the idea of combining option 1 and 2 but would insist on the extra orphan type and would suggest to rename it to something like "archived Trick".
Like this we could exclude them of the main Tricklist by default and show them depending on some filters... (only archived, all tricks including archived ...)
This would be similar like we handle this now with deleted tricks, to not break anything when deleting a trick (like the ComboDetails of a combo including that trick) we just mark them as deleted and down't show them anymore in the TrickList.

Composite Key / Compound Index

Nice, I like it. The >1000 was supposed to be only tempory.

Doubles, Variations, Different starting/ending Possition

Changing the Endpossiton of a trick makes it to a variation of that trick in my eyes, same as doubles...
I guess we could combine all these 3 things under Trick-Variations, either in the same database table or a separate one.

To handle that nicely in the google sheet, I would suggest to:

1. have a different document including all the "special" variations, which will have a different description or different video link, etc.

2. add a Variation column to the document of the main tricks, which includes specific keywords (with separators) like
   *[Number] for including double, triples, quads ,... to up this number
   diff[Number] a separate difficulty
   [position]->[position] different
   ...

   What are good separators for this? (We would need to separate text into a list, containing lists)

3. store variations to an extra database table, not containing full trick objects but only the changed values from the original trick, inheriting the other attributes

4. when parsing the documents automatically create the variations from the keywords of 3.

5. only linking the variations to the "orginal" trick.
   This would make it more expensive querying for variations but also easier to handle user added variations

This is just out of the top of my head, feel free to think trough all that again ;)

Dexie?

Now when we do this whole refactoring we could also consider changing to different library then Dexie...
I don't know what alternatives there are, but one problem we always had with Dexie, was the lack of a simple relational database.

Or should we also consider a different storage option?
How could we get more persistence?

[bastislack]

On the question of whether the Orphan-Type is needed: Firstly I think there is a small bit of confusion around what the Orphan-Type actually is:

Ok, thanks for the clarification :)

How would you track if a Trick has been "orphaned"?

I think this is basically answered in your first very detailed post in the image describing the process of deleting tricks for the different options. First we store the ids of predefined tricks in the current version, then we check the ids of the updated version and whichever tricks are missing in the updated version will be turned into 'Orphan Tricks'. Whenever the user first visits this trick a pop-up will show up asking the user whether to delete or keep the trick. It will then either be deleted or turned into a user trick.

You also convinced me that it is not the best idea to go with the magic number 10.000 :D So let's use the composite key variant.

Alternatively… Another Table could be created that defines all possible alternative combinations.

This seems like a good option to me. I also like @weberax's approach for that. However, I am still not sure how to handle variations (showing them on the main trick list or not; creating separate tricks for them or just list them in the original trick if they are not important; which ones should exist in the app anyway?). Do you think this is a decision we can delay a bit or should we come up with a way to do it now?

Since the isImportant field seems to be always be described by variationOf.length == 0 I think the whole field could be droppend an only the variationOf field be kept.

While it's true for now that I only put entries in rows where isImportant == false I am not sure if it will stay like this. E.g. a Double Yoda Roll 180 could be a trick that deserves a spot on the main trick list but is also a variation of a Double Yoda Roll. Again still uncertain about the variations topic...

So if something funky should happen between tricks, the user can just add it as text and it shows up similar to a trick in the UI

That seems like an easy fix for the problem. Maybe I wouldn't say that a combo is made out of tuples but rather that we can use 'normal' tricks and something like temporary/quick/fill trick (don't know how to best name it). Not really different from a note but kind of handled like a trick (just without the link to a trick page and so on).

Or should we also consider a different storage option?
How could we get more persistence?

After a quick search it looks to me like indexedDB is the best option for offline storage in PWAs and Dexie is the most well-maintained indexedDB wrapper as far as I know. So maybe we should just stay with Dexie. But if you find any better alternatives we can also change this.

[JulianDietzel]

@WaldemarLehner and I looked at the recent suggestions and updated our old graphics accordingly:

Changes

• We considered the idea of having a Variations-Table. In the process we discovered though, that it would be an almost exact reimplementation of the Tricks table. Also we ran into design issues because it would not be clear anymore which id actually specifies a trick. A variation would then have to consist of not just the base trick id but also its own id. We decided that just adding the variations to the Tricks table is probably the best way to go. To make this possible we made all attributes, except for id, trickStatus, technicalName and DateAddedEpoch optional. Non of the other attributes are strictly necessary and by making them optional we make it possible for variations to only override certain attributes.
• Create Video class to store URL, start- and end time. This will be stored as a JS-Object in the database. This a) seemed cleaner in general and b) solves an override issue with variations: If we only replace values that are not undefined, in the current system, it could happen that we replace the URL but leave the start and end timers from the original video without intending to do so. The class always groups the URL and the timings together.
• Renamed Orphan to Archived
• Renamed Predefined to Official and Custom to UserDefined (let us know what you think of this?)
• Split the ParentType Enum into two Enums EntityType and EntityStatus. The two Enum types didn't actually belong together. As a result the metadata table now has a triple composite primary key. This isn't ideal, but we still consider it to be better than before.
• Added DateAddedEpoch which defines the date that the trick was added as a UNIX epoch (this later allows for highlighting new tricks)
• Renamed isImportant to showInSearchQueries

Open Questions

• It would be quite easy to add multiple videos to a trick or a combo. Should we allow for this functionality or keep it simple?
• The enum names are rather... lacking, to say the least :D. Do you have a better name instead of "Entity"?
• We could not agree on or find a clean way to handle How can we handle tricks like 'Handpop to Nevermind'? #170 yet. We might want to first clearly define what it is exactly that we want to do.

Others

• We both did a quick search for Dexie alternatives but as much as we would both love to actually have a proper relational database (embedding MySQL with Webassembly doesn't quite seem appropriate ;D) Dexie seems to be the best option we have.

This is what we have for now. Let us know what you think or if we missed anything (for which there is a decent chance).

[bastislack]

Nice, it looks really good!

1. I like the way you want to handle variations.
2. The Video class is a good idea
3. Archived sounds good
4. Official and user defined sounds good
5. Splitting the enum sounds good.
6. DateAddedEpoch sounds good.
7. showInSearchQueries is good.

Open Questions:

1. Multiple videos per trick seems like a good idea.
2. We could use 'Stickable' instead of 'Entity'. Because we can stick tricks and combos ;)
3. We decided to not include such generic tricks in the app for now.
4. I think Dexie will do for now.

[JulianDietzel]

The database of the rewrite will be designed in accordance to this discussion. There will therefore be no change to the database of the current version of the app. It will be rewritten entirely as part of the complete rewrite of the app.