Convert media store to Pinia

[obulat]

Fixes

Fixes #1042 by @obulat
Fixes #1028 by @obulat

Description

This PR converts the Media store from Vuex to Pinia, and typescriptifies it.
It replaces the types that were previously set to be used from JS with TS generics. There are still a couple of places where I couldn't fix types, and added ts-ignore to be able to commit and push the changes.

Testing Instructions

Run the app. Searching all media types, filtering and loading more and viewing the detail pages should all work.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[sarayourfriend]

I opened a PR with some suggested additional simplifications to the media service alongside the ones you've already made: #1173

This also removes the redundant MediaDetail type (it duplicates the Media type and does so at the cost of type resolution performance as well).

[sarayourfriend]

Can we put the MediaState type in this file instead of delocalizing it? Kind of the same issue we have with moving props into separate files.

[sarayourfriend]

Stuffing all our media results into a single store seems like it's going to introduce some complications for types and also long term maintainability... it increases the complexity a lot. This store is responsible for managing the results for each individual media type as well as managing the results for the aggregating the results for the all view. I wonder if we can split these responsibilities, remove the state nesting, and fix some of the type errors along the way.

This is probably for a separate PR/issue, not this one!

If we changed the media store to be generic, something like this:

const createResultsStore = <T extends SupportedMediaType>(mediaType: T) => createStore(`${mediaType}-results`, () => {
  const state: ResultState<DetailFromMediaType<T>> = reactive({
    results: // unnested,
    fetchState: // unnested,
    // no need for audio or image key at the root
  })

  const getItemById = (id: string) => {
    return state.results.items[id]
  }

  const resultList = computed(() => Object.values(state.results.items))

  const resultCount = computed(() => state.results.count)

  // etc...
})

Then useImageStore and useAudioStore are separate entities. A third useAllResultsStore would aggregate the two (and any future stores) in a generic fashion.

This would remove all the [mediaType] keying we have to do now as well as the conditionals around ALL_MEDIA that we have to do now.

The search page itself would decide which store to use based on the search type, rather than having the media store opaquely decide which results to return.

Does this seem possible to you based on your understanding of the current stores?

[obulat]

This looks like a great idea, @sarayourfriend ! I will have to think more about it.
I was planning to extract something like singleMediaStore for the detail pages, and the relatedMediaStore(converted from composable), after Pinia conversion is done.

[obulat]

I opened a new issue for splitting the store based on @sarayourfriend 's comment, #1192 .

[sarayourfriend]

Looking great!

[sarayourfriend]

I think some of these are inaccurate, either they're not optional properties (but actually required and nullable instead) or they're always present but sometimes just empty (like peaks is a required property but is sometimes an empty array).

It's not a big deal though, in particular with the nullability, it'll just make writing code using these types more annoying because we'll have to add null checks in places that we don't need them (like on the array types).

thumbnail can be moved up to Media too as it's on both Audio and Images.

[sarayourfriend]

@obulat I opened #1186 with suggestions for how to avoid the create* pattern using mocks. Sorry I didn't get that to you yesterday.

[dhruvkb]

This is a huge refactor! Thanks for adding/updating tests for the additional confidence that things are running as they should.

[zackkrida]

Can we remove the comment above describing the finished state, if it no longer exists?

 * - `finished`: for multi-page requests, this is true when no more pages are left.

[zackkrida]

Suggested change

	* Decodes the text data to avoid encoding problems.
	* Also, converts the results from an array of media objects into an object with
	* media id as keys.
	* Decodes the text data to avoid encoding problems.
	* Also, converts the results from an array of media
	* objects into an object with media id as keys.

minor, minor nit to clean up the line breaks here

[zackkrida]

Suggested change

	}
	}

Remove whitespace for consistency with the method above it

[zackkrida]

I am sure a lot of the complexity came much earlier in the process, but I actually found this quite easy to review! It's great to see such a significant piece of code moved over to Pinia.