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

Refactor the way items are handled in Vuex stores #801

Closed
heyhippari opened this issue Feb 26, 2021 · 10 comments · Fixed by #2201
Closed

Refactor the way items are handled in Vuex stores #801

heyhippari opened this issue Feb 26, 2021 · 10 comments · Fixed by #2201
Labels
confirmed This bug was reproduced successfully by multiple people enhancement New feature or request frontend:store Pull requests or issues that deal with stores

Comments

@heyhippari
Copy link
Contributor

heyhippari commented Feb 26, 2021

Multiple design issues plague our current Vuex stores.
Among them is the way we store items coming from the API ("items" here refers to BaseItemDto, UserDto and PublicSystemInfo, the basic item types of the API)

We currently store multiple copies of these in various stores, which makes no sense (It creates more noise, makes store testing more complex and makes updates through the Websocket completely nonsensical, since they have to be done everywhere...)

Proposal

The proposed schema (containing only the relevant fields for each store) is as follows:
image

Some corrections apply to this schema, however:

  • In the Users store, publicUsers should be of the type string[]
  • The HomeSections store should be renamed to HomeScreen, and In the HomeSections store, sections should be of the type string[] and another property sectionContent of type Record<number, string[]> should be added. An enableHeader flag should also be part of the store.

Explanations

Core stores

The Servers, Users and Items stores are the central stores of the client. They store "raw" objects from the API in their respective types: ServerInfo, UserDto and BaseItemDto (Note: ServerInfo is built in the Vue client, but it acts as a core data type here).

These stores get referenced from other stores as the main source of data.

Related:

Session

This store is a combination of a few of our current stores: userViews, user and part of server. It's goal is, as the name implies, to represent the current user's session, with everything needed for it (It's sort of a "fit-all" store for things that don't really need their own stores)

It currently contains:

  • currentServer (string) - Contains the ID of the currently connected server
  • accessToken (string) - Contains the "raw" access token for the API
  • userViews (string) - Contains the ID of the BaseItemDto representing the user's libraries

Both currentServer and userViews are primarily accessed through getters which consolidate the list of IDs with the contents of the respective stores.

HomeSection

In order to be more modular, this store is completely refactored.

It now features a list of strings (sections), representing the various section types that make up the user's home screen.

Another property, sectionContent, contain the index of the section, as well as a list of IDs for the items in that section.

Finally, an enableHeader boolean sets the visibility of the HomeHeader component. An enableHeader getter checks if the first index of sections is the HomeHeader.

ShowSeasons

This store contains the relationships between BaseItems representing a show's seasons and episodes.

showSeasons contains a dictionary where keys represent the ID of the show and the value is a list of strings containing the IDs of the seasons.

seasonEpisodes contains a dictionary where keys represent the ID of the season and the value is a list of strings containing the IDs of the episodes.

Libraries

librariesContent contains a dictionary where keys represent the ID of the user view and the value is a list of strings containing the IDs of the contents.

librariesPrefs contains a dictionary where the keys represent the ID of the user view and the value is a DisplayPreferenceDto object containing that library's display preferences.

Note: To properly support folder-based libraries, a parentChild (or similar) property might be needed, in order to be able to build a proper tree of items to navigate.

PlaybackManager

queueItems would be changed to contain a list of strings representing the IDs of items in the queue.

To simplify the way shuffling and the queue in general is handled, a queueOrder property should be added, which by default would contain the indexes of items in queueItems.

By default, for a normal queue, this property would look like this: [0, 1, 2, 3, 4, ..., n], each entry pointing to an index in queueItems.

When shuffling, queueItems would remain untouched. Only queueOrder would be shuffled. Unshuffling the queue would then only reset the array to be a sequence of numbers.

When adding an item to a shuffled queue, the store would proceed as such:

  • Insert the new item at the end of queueItems
  • Insert the index of that item where needed in queueOrder

When adding an item to an unshuffled queue, the store would proceed as normal (Adding the item where needed, then re-generating the order).

@heyhippari heyhippari added enhancement New feature or request frontend:store Pull requests or issues that deal with stores labels Feb 26, 2021
@ferferga
Copy link
Member

Two things:

• I wouldn't put the enableHeader flag in this store. As it's done in #632, I think it's better to have our custom preferences in a separate store and leave the other stores to "expand" those settings.

That we we have a clear distinction between our custom prefs, the API supported ones and the session-based stuff.

• Don't know which name we could use, but if we plan to follow @Perseusdehond design concepts, we're going to have multiple "homepages", one for each content type that the server has available. So we should reconsider the HomeSections name.

@heyhippari
Copy link
Contributor Author

heyhippari commented Feb 26, 2021

I wouldn't put the enableHeader flag in this store. As it's done in #632, I think it's better to have our custom preferences in a separate store and leave the other stores to "expand" those settings.

True, though for organization it makes a bit more sense there. I was seeing the DisplayPreferences store as more of a "setting that applies to the general way the client behaves", with more specific settings being part of other more specific stores.

Thinking more about it, we should probably treat the HomeHeader has just another section, for it to really make sense. The only special thing would be that it's ALWAYS at ID 0 in sections. enableHeader then becomes a getter which only checks if ID 0 is the home header or not.

This also has the benefit of moving the contents of HomeHeader outside of the component and into the proper store.

Don't know which name we could use

I think HomeScreen or HomeSections fits. I see Perseus' pages (if we go that route) as "landing pages" or "content discovery pages", not home pages per se.

@ferferga
Copy link
Member

ferferga commented Feb 27, 2021

TLDR of some of the stuff @ThibaultNocchi and me threw at the air while talking about the store situation (specially while discussing #632), so they don't end up missing:

• Item-agnostic stores for API items (exactly as it's proposed in this issue, but you actually formalised it :D).

• DisplayPreferences store that saves the few API-supported fields that the API has (like Chromecast version, view types, etc etc), syncs and diffs between local and remote state on disconnections and casts custom preferences to the Javascript datatypes.

• Settings store for simple data (colors, themes, seek length... Booleans, integers, etc). Although API only supports strings for this, the state is fully casted and typed.

(All of this, except the API-supported fields of the DisplayPreferences API and diff syncing is already implemented in #632. The approach follows a similar fashion to what you propose with items, but for settings: "core" that handles raw API stuff and "secondary" for abstracted data)

• Specific stores for more complex data like arrays and dictionaries and full UI-wise features (like home sections). This is exactly another thing you're formalising here.

I think we all agree somewhat in the architecture we are aiming for, however there are a few key challenges we have with this API and I think we're missing, specially if we want to develop the connection between DisplayPreferences - "Core" stores or DisplayPreferences - Settings stores:

• How are we going to store data that mixes API-supported fields and not API-supported fields?

You had a really nice suggestion for handling the enableHeader field, and I like it so, beforehand, I agree we go with it 👍🏻. I will use it as an example of an hypothetical problem we might face (and that's why I asked about having it in the settings store instead).

For Homesections, we can use viewType in API to abstract the layout that the store saves with an id for each library type. However, how do we include the enableBoolean property that doesn't exist in the API?

We should think about how we're going to structure that kind of data, because I discovered how limited are we API-wise while working in that PR. We store them in the relevant store (alongside with complex data) and then "divide" them later and "redirect" it properly to where it fits? Or leave them separated "logically" but correctly placed so they don't need to be destructured?

This are concerns that we can decide to ignore though, as we could simply serialize everything to JSON and store the JSON string in the server. In fact, we almost went that route in #632 to avoid casting issues, until we managed to get it to work in an state that's practically impossible to have issues while casting.

We finally went the way of casting and doing things right API-wise (although that meant doing workarounds) as the data is more manageable in JF database and having a mediocre schema doesn't mean we should stick with the mediocrity, specially when we want to get rid of JSON blobs in the library database completely.

@ferferga
Copy link
Member

I think HomeScreen or HomeSections fits. I see Perseus' pages (if we go that route) as "landing pages" or "content discovery pages", not home pages per se.

I don't understand 😅. If you consider them landing pages (that's the best term to describe them, yeah, couldn't find it before ^^) what's the point of keeping Home in the name and not something like UI or Pages instead?

@heyhippari
Copy link
Contributor Author

I think HomeScreen or HomeSections fits. I see Perseus' pages (if we go that route) as "landing pages" or "content discovery pages", not home pages per se.

I don't understand sweat_smile. If you consider them landing pages (that's the best term to describe them, yeah, couldn't find it before ^^) what's the point of keeping Home in the name and not something like UI or Pages instead?

My "HomeSections" store is specifically for the actual home page :p

@ferferga
Copy link
Member

So, how are we going to store that data then, if we plan to follow his design approach?

@heyhippari
Copy link
Contributor Author

For #498 specifically, we can just assume that, depending on the type of home section, index 0 is a special item (The library, the collection, the actor, etc).

This keeps a consistent way to store data for everything, which doesn't involve having to make (and handle) different data for all the different sections. When we render that section, we just treat 0 as a special item, but most of the logic stays the same.

However, how do we include the enableBoolean property that doesn't exist in the API?

This is mostly an helper for the home header, specifically. There is an API for storing home sections which would be used for handling the list of actual sections. We don't have to store booleans for each section, fortunately :)

@stale
Copy link

stale bot commented Jun 3, 2021

Issues go stale after 60 days of inactivity. Mark the issue as fresh by adding a comment or commit. Stale issues close after an additional 14 days of inactivity. If this issue is safe to close now please do so. If you have any questions you can reach us on Matrix or Social Media.

@stale stale bot added the stale This issue or PR hasn't been updated in at least 60 days label Jun 3, 2021
@ThibaultNocchi
Copy link
Member

Sorry bot

@stale stale bot removed the stale This issue or PR hasn't been updated in at least 60 days label Jun 3, 2021
@stale
Copy link

stale bot commented Aug 2, 2021

Issues go stale after 60 days of inactivity. Mark the issue as fresh by adding a comment or commit. Stale issues close after an additional 14 days of inactivity. If this issue is safe to close now please do so. If you have any questions you can reach us on Matrix or Social Media.

@stale stale bot added the stale This issue or PR hasn't been updated in at least 60 days label Aug 2, 2021
@ThibaultNocchi ThibaultNocchi added confirmed This bug was reproduced successfully by multiple people and removed stale This issue or PR hasn't been updated in at least 60 days labels Aug 2, 2021
@ferferga ferferga unpinned this issue Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed This bug was reproduced successfully by multiple people enhancement New feature or request frontend:store Pull requests or issues that deal with stores
Projects
None yet
3 participants