LB-1761 : Save Fresh Releases filters in browser storage

[Mshahnawaz1]

Problem

Currently as discussed in ticket LB-1761 we don't store the state of filter in fresh-releases so when user refreshes the page all the choices of user get reset.

Solution

So to solve this issue we are storing the filter state in local storage using localForage library.
In this we are storing these states:

• checkedList,
• releaseTagsCheckList,
• releaseTagsExcludeCheckList,
• includeVariousArtists,
• coverartOnly,

We are using a custom hook userfilterpersistence.ts created in listenbrainz/hooks/userfilterpersistence.ts to define the operations for filter data.

Action

This needs to be checked if all the parameters that needs to be stored are correct. Also filters gets reset when we toggle from All to For you and also when refreshed.

[MonkeyDo]

The name here is a bit too generic, you can imagine there are other places in the codebase where we might have filters.
Maybe useFreshReleasesFilterPersistence ?

[MonkeyDo]

This seems like a cumbersome way to go about saving the filters state, and makes it harder to extend in the future.

I think the hook should instead expose a freshReleasesFilters object with all the filters, a setFreshReleasesFilters function that accepts an object of type StoredFilters, as well as your existing clearSavedFilters .

What this means is moving the state management of these filter options inside this hook component instead of in the ReleaseFilters component instead of passing the values and their respective setters to the hook.

In ReleaseFilters you can use the values exposed by the hook, something like :

const { freshReleasesFilters, setFreshReleasesFilters, clearSavedFilters } = useFilterPersistence();
const {
    checkedList,
    releaseTagsCheckList,
    releaseTagsExcludeCheckList,
    includeVariousArtists,
    coverartOnly
  } = freshReleasesFilters

---

The other approach is to create a hook that only persists one value, and use that hook for each filter value instead of useState.
Something along the lines of

function usePersistentState(key, defaultValue) {
  const [value, setValue] = useState(() => {
    const storedValue = // get value from localForage here
    return storedValue ?? defaultValue;
  });

  useEffect(() => {
     // set value using localForage here
  }, [key, value]);

  return [value, setValue];
}

And then in ReleaseFilters:

const [checkedList, setCheckedList] = usePersistentState(`${RELEASE_FILTERS_STORAGE_KEY}-${pageType}-checkedList`, []);

Which might be easier to use, but does mean as many localForage calls as we have options, when we load the component. I would say test it out and see if there is an impact on performance at all?

[MonkeyDo]

This syntax can be made a lot shorter like so to filter out falsy values (will also filter out empty strings but that's good in this case):

Suggested change

	checkedList: checkedList.filter(
	(item): item is string => item !== undefined
	),
	checkedList: checkedList.filter(Boolean),

[MonkeyDo]

Here is the core of your issue with the filters resetting when you change between "for you" and general releases.

I think the ideal solution is to save the settings separately (more flexibility for users), using this key as a prefix, and adding the pageType as a suffix, something like:

key = `${RELEASE_FILTERS_STORAGE_KEY}-${pageType}`;

Which will result in the keys "release-filters-user" and "release-filters-sitewide" to store the filters separately.

[Mshahnawaz1]

Thank you @MonkeyDo for such detailed explanation. Will do the necessary changes.😃

[MonkeyDo]

Hello @Mshahnawaz1 !
Are you able and willing to finish this PR? If not, please let me know and I will take it over to finish it up.

[Mshahnawaz1]

Hello @Mshahnawaz1 ! Are you able and willing to finish this PR? If not, please let me know and I will take it over to finish it up.

Hi @MonkeyDo !
I have been busy in some academic works and will not be able to continue working on this.
Sorry for the trouble, hope you don't mind.
Also thank you for your guidance all along.