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

Store category sorting for recent, favorites and uncategorized per account #1169

Open
stefan-niedermann opened this issue Apr 23, 2021 · 7 comments

Comments

@stefan-niedermann
Copy link
Member

Currently only "real" categories are stored per account, while the sorting order for "meta" categories is stored globally.
→ Store category sorting method for RECENT, FAVORITES and UNCATEGORIZED per account.

@AlpAcA0072
Copy link

My understanding is that defaultly there's three categories named RECENT, FAVORITES and UNCATEGORIZED store the corresponding notes. It is right?

@AlpAcA0072
Copy link

I'm not sure about the meaning of "real" category and "meta" category, could you please explain it?

@stefan-niedermann
Copy link
Member Author

Let me explain:

Current state

Each note can have a category, like Music or any other text. Information to those "real" categories (like the sort order) is stored in the CategoryOptions table.
Additionally we have some "meta" categories, like FAVORITES, RECENT and UNCATEGORIZED - you can see them in the sidebar and selecting one of them will only show notes from this "meta" category. The difference is, that the relation between those meta categories and each note is not explicitly stored - For example: Selecting all notes of FAVORITES will display all notes (no matter in which real category they are), if they have FAVORITE property set to true.

One can also sort those meta categories (lexicographical or by modified date), just like the real categories, but the data is not stored in the CategoryOptions table, because only real categories belong into that. Instead they are stored in SharedPreferences. A simple key / value store instead of a database table, because we only have a fixed set of meta categories.

While everything is working fine with real categories, the meta categories are currently not aware of multiple accounts - in our key / value store we only have for example:

key value
FAVORITES modified
RECENT modified
UNCATEGORIZED lexicographically

Goal of this issue

  1. Do not modify anything regarding real categories - they work as they are supposed to.
  2. Make meta categories account aware, target state in our key / value store could look for example like this:
key value
FAVORITES_1 modified
RECENT_1 modified
UNCATEGORIZED_1 lexicographically
FAVORITES_2 modified
RECENT_2 modified
UNCATEGORIZED_2 lexicographically

(where _1 and _2 are referring to the id of the account

  1. Make sure that reading and updating works properly. Also handle the deletion of an account where we need to take care about removing the corresponding entries for the deleted account.
  2. Migrate the existing meta category information with a database migration

@AlpAcA0072
Copy link

AlpAcA0072 commented May 7, 2021

Thank you very much for your detailed explanation!

@AlpAcA0072
Copy link

AlpAcA0072 commented May 13, 2021

It seems that if we want to make the meta categories account aware, we need to create a new constructor in NavigationCategory.java and modify all method calling associated with it. If we only modify the meta categories in reading, updating and deletion by imitating the real category, it will brings about a series of problems, because the accountId of meta catogory is defaultly set as Long.MIN_VALUE. Is there any possible solution for this problem?

AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 15, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 15, 2021
@AlpAcA0072
Copy link

AlpAcA0072 commented May 15, 2021

@stefan-niedermann Hello, I'm working on this issue right now. I have some progress and I want to know if they are feasible.

  1. For reading, I add the accountId to the end of prefKey so that the key of meta category is account aware.
  2. For updating, I add the accountId to the end of sp.putInt() so that we won't miss the attached account.
  3. For deletion, I'm not sure about what to do, because it seems there's nothing associated with the category, even if the real category.
    I would be very much appreciate it if you could give me some advice! 🙂

@stefan-niedermann
Copy link
Member Author

Hey 👋 you're on a good way. Please open a Pull Request where we can see the diff and discuss the implementation details.
For 3. you won't see anything associated with categories because real categories are part of the notes and connected with foreign keys to the accounts - deleting an account in the database will cascade and automatically delete the real categories. For meta categories such an automatism does not work because the are not stored in the database but in shared preferences - that's the reason why we need to delete them manually when deleting an account.

AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 16, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 16, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 16, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 16, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 16, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 18, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 18, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 18, 2021
AlpAcA0072 added a commit to AlpAcA0072/nextcloud-notes that referenced this issue May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants