Migrate to new group id - rebase #1214

Altonss · 2023-02-07T18:20:34Z

This PR solves #1045

Fix typo

Add ID column to tmpDbGroups

Move to group.name in DatabaseTest.java

Migrate to group.name in DatabaseTest.java

…t...

…unit test..." This reverts commit ed724b8.

Altonss · 2023-02-07T20:22:41Z

I must have done something wrong, but now the tests are failing :( ping @TheLastProject

app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java

app/src/main/java/protect/card_locker/MainActivity.java

…java Co-authored-by: Sylvia van Os <[email protected]>

TheLastProject · 2023-02-14T22:16:54Z

I found the reason behind a few of the failing tests. Not all yet, there's some database constraint that are failing. Need to look at that still.

Altonss · 2023-02-14T22:31:25Z

I found the reason behind a few of the failing tests. Not all yet, there's some database constraint that are failing. Need to look at that still.

Thanks a lot for the help! Now it only fails at ImportExport at 2 places...

TheLastProject · 2023-03-08T20:08:27Z

app/src/main/java/protect/card_locker/importexport/CatimaImporter.java

        List<Group> cardGroups = DBHelper.getLoyaltyCardGroups(database, cardId);
+
+        Integer groupId = CSVHelpers.extractInt(DBHelper.LoyaltyCardDbIdsGroups.groupID, record, false);
        cardGroups.add(DBHelper.getGroup(database, groupId));


Finally figuring out why one of the tests is failing.

For one of the unit tests, this adds an entry to cardGroups with value null, causing a NullPointerException in setLoyaltyCardGroups.

This is because DBHelper.getGroup() has 0 results for groupId 1.

I'm not quite sure yet why.

Thanks a lot for figuring this out! Hmm I don't know why either... 🤔

Is maybe importCardGroupMappingV2 used instead of importCardGroupMappingV3, which defaults to null if no string found?

Doesn't look like it, the failure happens in importCardGroupMappingV3. It's the multipleCardsExportImportWithGroups test failing.

E/Catima: Failed to import data java.lang.NullPointerException at protect.card_locker.DBHelper.setLoyaltyCardGroups(DBHelper.java:641) at protect.card_locker.importexport.CatimaImporter.importCardGroupMappingV3(CatimaImporter.java:540) at protect.card_locker.importexport.CatimaImporter.parseV3CardGroups(CatimaImporter.java:361) at protect.card_locker.importexport.CatimaImporter.parseV3(CatimaImporter.java:211) at protect.card_locker.importexport.CatimaImporter.importCSV(CatimaImporter.java:88) at protect.card_locker.importexport.CatimaImporter.importData(CatimaImporter.java:59) at protect.card_locker.importexport.MultiFormatImporter.importData(MultiFormatImporter.java:47) at protect.card_locker.ImportExportTest.multipleCardsExportImportWithGroups(ImportExportTest.java:468) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:580) at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:287) at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829)

obfusk · 2023-05-31T19:38:54Z

app/src/main/java/protect/card_locker/MainActivity.java

-                    bundle.putString(LoyaltyCardEditActivity.BUNDLE_ADDGROUP, groupsTabLayout.getTabAt(selectedTab).getText().toString());
+                TabLayout.Tab tab = groupsTabLayout.getTabAt(selectedTab);
+                if (tab != null) {
+                    Group group = (Group) groupsTabLayout.getTabAt(selectedTab).getTag();


Am I missing something here or was this unnecessary duplication?

Suggested change

Group group = (Group) groupsTabLayout.getTabAt(selectedTab).getTag();

Group group = (Group) tab.getTag();

obfusk · 2023-05-31T19:40:09Z

app/src/main/java/protect/card_locker/importexport/CatimaImporter.java


-        DBHelper.insertGroup(database, id);
+    /**
+     * Import a single group from the V3 scheme (database v16) into the database using the given


Isn't that v17?

obfusk · 2023-05-31T21:01:19Z

app/src/main/java/protect/card_locker/importexport/CatimaImporter.java

        List<Group> cardGroups = DBHelper.getLoyaltyCardGroups(database, cardId);
+
+        Integer groupId = CSVHelpers.extractInt(DBHelper.LoyaltyCardDbIdsGroups.groupID, record, false);
        cardGroups.add(DBHelper.getGroup(database, groupId));


Not 100% sure this is correct, but it makes the tests pass.

Suggested change

cardGroups.add(DBHelper.getGroup(database, groupId));

cardGroups.add(DBHelper.getGroupByName(database, groupsTable.get(groupId)));

Also not sure how importing works. Does it keep existing cards and groups? If so does this handle conflicting IDs between existing data and the exported data?

Might be better to have a DBHelper.insertGroup(database, id, name) to be used in importGroupV3() to insert a group with a specific ID.

obfusk · 2023-06-01T18:24:46Z

@Altonss @TheLastProject I opened #1333 based on this PR.

Altonss and others added 30 commits February 7, 2023 19:00

First changes to DBHelper to adapt to the new group id

53986f8

Update DBHelper.java

5723f83

Fix typo

Improve DBHelper and adapt Group.java to id changes

005f984

Fix group issues in importer

41367b2

Update ManageGroupCursorAdapter to group id

e29423f

Update CatimaExporter and LoyaltyCardViewActivity.java

dac662c

Further update to group id

f09b5b0

Clean up ManageGroupsActivity.java and update tests to new group id

c05fad8

Fix ManageGroupsActivity.java because of merge conflicts

b35c6fc

Migrate CatimaImporter to group id scheme

ca8aa67

Migrate CatimaImporter to group id scheme

13513ef

Fix updateMissingGroup test

290329e

Fix addRemoveOneGroup test

cee30cd

Fix typo in DBHelper migration to v16 database

bcae682

Fix the join for database version migration

2f5c51e

Update DBHelper.java

b2f3748

Add ID column to tmpDbGroups

Update DatabaseTest.java

6fbbec8

Move to group.name in DatabaseTest.java

Update DatabaseTest.java

42d2e6c

Migrate to group.name in DatabaseTest.java

Fix DatabaseTest.java

22598a7

Fix MainActivity.java : group._id was still used instead of group.name

64b6972

Fix importGroup

ee3126b

Add extra check to not insert new group with duplicate name

d7c0b64

Add extra check to not insert new group with duplicate name

7127354

Fix group id that was still used in GroupCursorAdapter instead of name

b1b3882

Fix handling of group in importCardGroupMappingV3

ccce7e5

Remove strangely failing test to see what happens during the unit tes…

d7571cb

…t...

Revert "Remove strangely failing test to see what happens during the …

04c73bc

…unit test..." This reverts commit ed724b8.

Rewrite some failing tests because of list order

e292d64

Remove not necessary order by group id

aad7648

Remove unused lines

387e46b

Altonss added 11 commits February 7, 2023 19:12

Fix version number in parseV3 error message

2eb2301

Improved importGroupV3 handling

51f6999

Add tests by group id

8559272

Remove useless groupName usage

32653d1

Add group order to export and import

de46f0b

Removed unused equals and hashCode methods in Group class

e5d4fcc

Revert back to the use of whereAttrs, withArgs

600a854

Check if the new name already exists before updating group

f174a61

Fix alphabetical ordering of groups

f17bdaa

Move addGroup to id

aab75f4

Move addGroup to id

e0afb47

Altonss mentioned this pull request Feb 7, 2023

Migrate to new group id #1046

Closed

Altonss added 2 commits February 7, 2023 19:32

Fix import

615fb96

Fix usage copyOf (too high api level)

5df9078

TheLastProject requested changes Feb 14, 2023

View reviewed changes

app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java Outdated Show resolved Hide resolved

app/src/main/java/protect/card_locker/MainActivity.java Outdated Show resolved Hide resolved

Update app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.…

bffe3d7

…java Co-authored-by: Sylvia van Os <[email protected]>

Altonss added 2 commits February 14, 2023 23:24

Fix getTabAt

af85386

Merge remote-tracking branch 'my-repo/group-id' into group-id

344b198

reuse same type as parseV2

7c0b611

TheLastProject reviewed Mar 8, 2023

View reviewed changes

obfusk reviewed May 31, 2023

View reviewed changes

obfusk mentioned this pull request Jun 1, 2023

DB: migrate to unique IDs for groups #1333

Draft

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to new group id - rebase #1214

Migrate to new group id - rebase #1214

Altonss commented Feb 7, 2023

Altonss commented Feb 7, 2023 •

edited

Loading

TheLastProject commented Feb 14, 2023

Altonss commented Feb 14, 2023

TheLastProject Mar 8, 2023 •

edited

Loading

Altonss Mar 8, 2023

Altonss Mar 8, 2023

TheLastProject Mar 14, 2023

obfusk May 31, 2023

obfusk May 31, 2023

obfusk May 31, 2023

obfusk May 31, 2023

obfusk May 31, 2023 •

edited

Loading

obfusk commented Jun 1, 2023

	Group group = (Group) groupsTabLayout.getTabAt(selectedTab).getTag();
	Group group = (Group) tab.getTag();

	cardGroups.add(DBHelper.getGroup(database, groupId));
	cardGroups.add(DBHelper.getGroupByName(database, groupsTable.get(groupId)));

Migrate to new group id - rebase #1214

Are you sure you want to change the base?

Migrate to new group id - rebase #1214

Conversation

Altonss commented Feb 7, 2023

Altonss commented Feb 7, 2023 • edited Loading

TheLastProject commented Feb 14, 2023

Altonss commented Feb 14, 2023

TheLastProject Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

Altonss Mar 8, 2023

Choose a reason for hiding this comment

Altonss Mar 8, 2023

Choose a reason for hiding this comment

TheLastProject Mar 14, 2023

Choose a reason for hiding this comment

obfusk May 31, 2023

Choose a reason for hiding this comment

obfusk May 31, 2023

Choose a reason for hiding this comment

obfusk May 31, 2023

Choose a reason for hiding this comment

obfusk May 31, 2023

Choose a reason for hiding this comment

obfusk May 31, 2023 • edited Loading

Choose a reason for hiding this comment

obfusk commented Jun 1, 2023

Altonss commented Feb 7, 2023 •

edited

Loading

TheLastProject Mar 8, 2023 •

edited

Loading

obfusk May 31, 2023 •

edited

Loading