brarcher / loyalty-card-locker

Stores your barcode-based store/loyalty cards on your phone
GNU General Public License v3.0
171 stars 29 forks source link

New feature: Favorite cards #369

Open t351206 opened 4 years ago

t351206 commented 4 years ago

Description (this is a follow-up to #305 ):

If you have some favorite loyalty cards it is useful to have them as first items in the listview. Therefore I added the "star-unstar" feature. Cards in the LoyaltyCardViewActivity can be be starred in order to make them a favorite card.

Therefore a new SQLite table column was added where the starring status is saved.

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.

t351206@gmx.de

t351206 commented 4 years ago

I also want to state that I'm missing tests for this starring functionality. @TheLastProject Which test do you think would make sense?

TheLastProject commented 4 years ago

I would like to see the following tests:

  1. UI shows correct buttons and buttons cause correct state change (once for starring and once for unstarring)
  2. Import CSV file with no star information
  3. Import with star information
  4. Import with invalid star information
  5. Main screen showing starred cards on top correctly
  6. Import from URI
  7. Import from URI with bad data

I think that should cover things

t351206 commented 4 years ago

@TheLastProject : Thanks for the test proposals!

ad 1) I think I need some help here. ad 2) see importWithNoStarredField ad 3) see multipleCardsExportImportSomeStarred and also implicitly included in other tests ad 4) see importWithInvalidStarField ad 5) see addFourLoyaltyCardsTwoStarred ad 6) no starStatus will be exported for single cards, starStatus will be automatically set to 0 for import; covered with existing test ad 7) I think this is an already existing issue. There is nothing implemented to avoid import of bad data (store, note, cardId, barcode can be null). I fixed it with an additional check if one of the four values is null. Furthermore I added two tests.

t351206 commented 4 years ago

How can I test clicking on the star?

shadowOf(activity).clickMenuItem(R.id.action_star_unstar) is producing an exception: java.lang.NullPointerException at org.robolectric.fakes.RoboMenuItem.setIcon(RoboMenuItem.java:107) at protect.card_locker.LoyaltyCardViewActivity.setStarInDB(LoyaltyCardViewActivity.java:370) at protect.card_locker.LoyaltyCardViewActivity.onOptionsItemSelected(LoyaltyCardViewActivity.java:359) at android.app.Activity.onMenuItemSelected(Activity.java:2914) at androidx.fragment.app.FragmentActivity.onMenuItemSelected(FragmentActivity.java:384) at androidx.appcompat.app.AppCompatActivity.onMenuItemSelected(AppCompatActivity.java:219) at org.robolectric.shadows.ShadowActivity.clickMenuItem(ShadowActivity.java:502) at protect.card_locker.LoyaltyCardViewActivityTest.checkPushStarIcon(LoyaltyCardViewActivityTest.java:718) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:600) at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) at org.robolectric.internal.SandboxTestRunner$2.evaluate(SandboxTestRunner.java:260) at org.robolectric.internal.SandboxTestRunner.runChild(SandboxTestRunner.java:130) at org.robolectric.internal.SandboxTestRunner.runChild(SandboxTestRunner.java:42) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268) at org.robolectric.internal.SandboxTestRunner$1.evaluate(SandboxTestRunner.java:84) at org.junit.runners.ParentRunner.run(ParentRunner.java:363) at org.junit.runner.JUnitCore.run(JUnitCore.java:137) at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68) at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33) at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230) at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)

t351206 commented 4 years ago

@TheLastProject: I think I solved the testing issue! Test "checkPushStarIcon" is working as expected!

TheLastProject commented 4 years ago

Sorry I didn't get back to you in your last reply, things have been a bit chaotic here and I forgot. Let me build your branch and play around with it a bit on my phone :)

TheLastProject commented 4 years ago

Have a few more comments but the code seems to work quite well and I do like it a lot :)

TheLastProject commented 3 years ago

Hi @t351206,

After talking with @brarcher I ended up forking Loyalty Card Locker. I would love to include your feature in my fork (which I will soon publish after I have a new icon). Could you please make this pull request on https://github.com/TheLastProject/Catima?