Store category sorting for recent, favorites and uncategorized per account

[AlpAcA0072]

Store category sorting for recent, favorites and uncategorized per account.

[AlpAcA0072]

I'm still working on deleting meta category when deleting an account.

[AlpAcA0072]

Reading and updating are realized as I said in the relevant issue.
As for deleting, I get the SharedPreferences of current context and uses sharedPreferences.remove() to remove the related value by the key.

[AlpAcA0072]

There's stiil a problem that I don't know how to test the modified code, so I check the keys mannually to make sure they are the same.
The NotesRepositoryTest is passed, however, it seems that the relevant tests contain no test about SharedPreferences or meta category.

[stefan-niedermann]

Yeah, we don't have that much tests (yet). I am working from time to time on the coverage, but that's a huge load of work, given that we started with 0 tests...

I have added a very basic unit test for the delete method of the NotesRepository (see master branch). You should be able to extend it for your use case after rebasing your fork or merging the latest master.

[AlpAcA0072]

Yeah, we don't have that much tests (yet). I am working from time to time on the coverage, but that's a huge load of work, given that we started with 0 tests...

I have added a very basic unit test for the delete method of the NotesRepository (see master branch). You should be able to extend it for your use case after rebasing your fork or merging the latest master.

Thank you very much for your hard work and feedback!
I've rebased my fork and merging the latest master to my branch, finished with the new test testDeleteAccount and passed all the tests in NotesRepositoryTest.java.

[stefan-niedermann]

I did an early review, and i guess you are overthinking it a bit, especially in the modifyCategoryOrder method... I don't really understand why you additionally call putString() and place some hard coded strings in the SharedPreferences...?

Also: A migration is missing to migrate the existing SharedPreferences to the account-specific ones. You can orientate on Migration_21_22.java for that.

[AlpAcA0072]

I’ve modified the code according to the above suggestions and passed the latest added tests. 😄

[AlpAcA0072]

I don't know exactly what to do in the migration. Should I refresh the sharedPreferences? Or should I uses the refreshed sharedPreferences to update the database using execSQL?

[stefan-niedermann]

You basically need to read all account IDs from the database, migrate sharedPreferences as described here and then remove the old sharedPreferences which were not account specific

[AlpAcA0072]

You basically need to read all account IDs from the database, migrate sharedPreferences as described here and then remove the old sharedPreferences which were not account specific

How can I know the structure of the table of database? Without knowing it I could not make a query.

[stefan-niedermann]

How can I know the structure of the table of database?

We are using the Room library as ORM. The entities are defined in NotesDatabase at the very top, each of the class mentioned there matches one table in the database. Each property of a class matches one column of the corresponding table.

Everything you need is to fetch the set of all account IDs and iterate it to add sharedPrefs for each of them.
A similar query is made in the Migration_22_23.

Don't forget to increment the database version, otherwise your migration will not be applied (i forgot it a few times myself and was frustrated because it didn't work 😉 )

[AlpAcA0072]

I think I've finished with that. Please check whether there is a problem.
Thanks a lot!😆

[AlpAcA0072]

How can I know the structure of the table of database?

We are using the Room library as ORM. The entities are defined in NotesDatabase at the very top, each of the class mentioned there matches one table in the database. Each property of a class matches one column of the corresponding table.

Everything you need is to fetch the set of all account IDs and iterate it to add sharedPrefs for each of them.
A similar query is made in the Migration_22_23.

Don't forget to increment the database version, otherwise your migration will not be applied (i forgot it a few times myself and was frustrated because it didn't work 😉 )

Do I need to add the modified sharedPreferences to the database? I've finished the Migration_23_24.java and want to know is there anything wrong with my implementation

[stefan-niedermann]

Do I need to add the modified sharedPreferences to the database?

No.

But you are removing the keys too early. This way it will only work for one set up account, not in a multi accoubt scenario.
You must only remove them after looping over all accounts.

Also please use always hardcoded strings in Migrations (instead of getString(R.string.xxx)) because the values might change over the time but need to stay the same in migrations forever 🙂

[AlpAcA0072]

Do I need to add the modified sharedPreferences to the database?

No.

But you are removing the keys too early. This way it will only work for one set up account, not in a multi accoubt scenario.
You must only remove them after looping over all accounts.

Also please use always hardcoded strings in Migrations (instead of getString(R.string.xxx)) because the values might change over the time but need to stay the same in migrations forever 🙂

If I hardcoded the string, for example "Uncategorized" to replace the R.string.action_uncategorized. For any languages other than English, how to guarantee that this method will also work?

[AlpAcA0072]

This version satisfy all the requirements above.😄

[stefan-niedermann]

You are absolutely right, what a pity that we use localized strings a keys.

We should move away from them during this migration and i unfortunately can not see any other option then gathering a list of all available translated values (see strings.xml files), put them into an array in the migration and iterate them.

As a new key you can use something meaningful like

• category_sorting_ + favorite_ + 1
• category_sorting_ (prefix to distinguish from other sharedPrefs) + favorite_ (meta category) + 1 (account id)

You can either store those new keys as

public static final String XYZ = "category_sorting_";

or alternatively in the values/strings.xml as

<string name="XYZ" translatable="false">category_sorting_</string>

Actually what you have found here is a bug from my point of view, because if one changes the language of his Android device, the sorting of the meta categories won't work.

Some extra miles to go, but worth it.

[AlpAcA0072]

You are absolutely right, what a pity that we use localized strings a keys.

We should move away from them during this migration and i unfortunately can not see any other option then gathering a list of all available translated values (see strings.xml files), put them into an array in the migration and iterate them.

As a new key you can use something meaningful like

• category_sorting_ + favorite_ + 1
• category_sorting_ (prefix to distinguish from other sharedPrefs) + favorite_ (meta category) + 1 (account id)

You can either store those new keys as

public static final String XYZ = "category_sorting_";

or alternatively in the values/strings.xml as

<string name="XYZ" translatable="false">category_sorting_</string>

Actually what you have found here is a bug from my point of view, because if one changes the language of his Android device, the sorting of the meta categories won't work.

Some extra miles to go, but worth it.

So actually what I need to do is to defined three untranslatable meta categories, and migrate the old one to the new one. Because the newly defined one is staticlly stored in sharedPreferences, we don't have to worry about internationalization.
Am I right?
That is such a wonderful idea!

[stefan-niedermann]

Yes, it can be considered being a bug to use translated values as keys (which we currently do). Therefore we should replace them with static values while we are migrating them anyway.

[AlpAcA0072]

I guess now we don't have to worry about hardcoded strings in Migrations. I added four elements to Strings.xml add using them in the Migration_23_24.java, does this meets the above requiements?

[AlpAcA0072]

@stefan-niedermann Do I have to resolve the conflicts shown below? Is there anything I can do anymore?

[stefan-niedermann]

I did not forget you, i am just quite busy with my work and my private life at the moment - sorrs for the delay.

[AlpAcA0072]

Thank you for your attention. I am also very busy these days and I hope the repair of the issue will not be delayed.