From ccf3d1f3d612e71794fd2047ccf89b2f0856245c Mon Sep 17 00:00:00 2001 From: Sylvia van Os Date: Tue, 31 Dec 2019 18:23:45 +0100 Subject: [PATCH 1/3] Fix converting loyalty card to barcodeless The LoyaltyCardEditActivity assumes on many places that an empty string means it doesn't know a value yet. This patch ensures that the BarcodeSelectorActivity returns a special string so that the LoyaltyCardEditActivity can distinguish explicitly picking no barcode from a not yet populated field. --- .../card_locker/BarcodeSelectorActivity.java | 2 +- .../card_locker/LoyaltyCardEditActivity.java | 79 +++++++++++-------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java b/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java index 18becdcc6..60ca02386 100644 --- a/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java +++ b/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java @@ -145,7 +145,7 @@ public class BarcodeSelectorActivity extends AppCompatActivity public void onClick(View view) { Log.d(TAG, "Selected no barcode"); Intent result = new Intent(); - result.putExtra(BARCODE_FORMAT, ""); + result.putExtra(BARCODE_FORMAT, LoyaltyCardEditActivity.NO_BARCODE); result.putExtra(BARCODE_CONTENTS, cardId); BarcodeSelectorActivity.this.setResult(RESULT_OK, result); finish(); diff --git a/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java b/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java index f5397409c..1ac8bb02d 100644 --- a/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java +++ b/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java @@ -35,6 +35,7 @@ import java.io.InvalidObjectException; public class LoyaltyCardEditActivity extends AppCompatActivity { private static final String TAG = "CardLocker"; + protected static final String NO_BARCODE = "_NO_BARCODE_"; private static final int SELECT_BARCODE_REQUEST = 1; @@ -229,42 +230,49 @@ public class LoyaltyCardEditActivity extends AppCompatActivity if(cardIdFieldView.getText().length() > 0 && barcodeTypeField.getText().length() > 0) { - String formatString = barcodeTypeField.getText().toString(); - final BarcodeFormat format = BarcodeFormat.valueOf(formatString); - final String cardIdString = cardIdFieldView.getText().toString(); - - if(barcodeImage.getHeight() == 0) + if(barcodeTypeField.getText().equals(NO_BARCODE)) { - Log.d(TAG, "ImageView size is not known known at start, waiting for load"); - // The size of the ImageView is not yet available as it has not - // yet been drawn. Wait for it to be drawn so the size is available. - barcodeImage.getViewTreeObserver().addOnGlobalLayoutListener( - new ViewTreeObserver.OnGlobalLayoutListener() - { - @Override - public void onGlobalLayout() - { - if (Build.VERSION.SDK_INT < 16) - { - barcodeImage.getViewTreeObserver().removeGlobalOnLayoutListener(this); - } - else - { - barcodeImage.getViewTreeObserver().removeOnGlobalLayoutListener(this); - } - - Log.d(TAG, "ImageView size now known"); - new BarcodeImageWriterTask(barcodeImage, cardIdString, format).execute(); - } - }); + barcodeImageLayout.setVisibility(View.GONE); } else { - Log.d(TAG, "ImageView size known known, creating barcode"); - new BarcodeImageWriterTask(barcodeImage, cardIdString, format).execute(); - } + String formatString = barcodeTypeField.getText().toString(); + final BarcodeFormat format = BarcodeFormat.valueOf(formatString); + final String cardIdString = cardIdFieldView.getText().toString(); - barcodeImageLayout.setVisibility(View.VISIBLE); + if(barcodeImage.getHeight() == 0) + { + Log.d(TAG, "ImageView size is not known known at start, waiting for load"); + // The size of the ImageView is not yet available as it has not + // yet been drawn. Wait for it to be drawn so the size is available. + barcodeImage.getViewTreeObserver().addOnGlobalLayoutListener( + new ViewTreeObserver.OnGlobalLayoutListener() + { + @Override + public void onGlobalLayout() + { + if (Build.VERSION.SDK_INT < 16) + { + barcodeImage.getViewTreeObserver().removeGlobalOnLayoutListener(this); + } + else + { + barcodeImage.getViewTreeObserver().removeOnGlobalLayoutListener(this); + } + + Log.d(TAG, "ImageView size now known"); + new BarcodeImageWriterTask(barcodeImage, cardIdString, format).execute(); + } + }); + } + else + { + Log.d(TAG, "ImageView size known known, creating barcode"); + new BarcodeImageWriterTask(barcodeImage, cardIdString, format).execute(); + } + + barcodeImageLayout.setVisibility(View.VISIBLE); + } } View.OnClickListener captureCallback = new View.OnClickListener() @@ -366,6 +374,13 @@ public class LoyaltyCardEditActivity extends AppCompatActivity String cardId = cardIdFieldView.getText().toString(); String barcodeType = barcodeTypeField.getText().toString(); + // We do not want to save the no barcode string to the database + // it is simply an empty there for no barcode + if(barcodeType.equals(NO_BARCODE)) + { + barcodeType = ""; + } + if(store.isEmpty()) { Snackbar.make(storeFieldEdit, R.string.noStoreError, Snackbar.LENGTH_LONG).show(); @@ -484,7 +499,7 @@ public class LoyaltyCardEditActivity extends AppCompatActivity } if(contents != null && contents.isEmpty() == false && - format != null) + format != null && format.isEmpty() == false) { Log.i(TAG, "Read barcode id: " + contents); Log.i(TAG, "Read format: " + format); From cc99af13e428127f8fe45833cae6f2566817035b Mon Sep 17 00:00:00 2001 From: Sylvia van Os Date: Wed, 1 Jan 2020 19:43:23 +0100 Subject: [PATCH 2/3] Fix tests --- .../card_locker/BarcodeSelectorActivityTest.java | 5 ++++- .../card_locker/LoyaltyCardViewActivityTest.java | 13 +++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java b/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java index 0d53e506f..e03cc216f 100644 --- a/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java +++ b/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java @@ -63,7 +63,10 @@ public class BarcodeSelectorActivityTest { // Clicking button should create "empty" barcode activity.findViewById(R.id.noBarcode).performClick(); Intent resultIntent = shadowOf(activity).getResultIntent(); - assertEquals("", resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_FORMAT)); + + // The BarcodeSelectorActivity should return the special NO_BARCODE string to differentiate + // from nothing being set yet + assertEquals(LoyaltyCardEditActivity.NO_BARCODE, resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_FORMAT)); assertEquals("abcdefg", resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_CONTENTS)); } diff --git a/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java b/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java index e0b6b7441..0a38c4452 100644 --- a/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java +++ b/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java @@ -126,7 +126,16 @@ public class LoyaltyCardViewActivityTest assertEquals(store, card.store); assertEquals(note, card.note); assertEquals(cardId, card.cardId); - assertEquals(barcodeType, card.barcodeType); + + // The special "No barcode" string shouldn't actually be written to the loyalty card + if(barcodeType.equals(LoyaltyCardEditActivity.NO_BARCODE)) + { + assertEquals("", card.barcodeType); + } + else + { + assertEquals(barcodeType, card.barcodeType); + } assertNotNull(card.headerColor); assertNotNull(card.headerTextColor); } @@ -539,7 +548,7 @@ public class LoyaltyCardViewActivityTest activityController.resume(); // Save and check the loyalty card - saveLoyaltyCardWithArguments(activity, "store", "note", BARCODE_DATA, "", false); + saveLoyaltyCardWithArguments(activity, "store", "note", BARCODE_DATA, LoyaltyCardEditActivity.NO_BARCODE, false); } @Test From ce81d3afd41c461a1065506f5de12dbd37cd14d1 Mon Sep 17 00:00:00 2001 From: Sylvia van Os Date: Thu, 2 Jan 2020 21:15:06 +0100 Subject: [PATCH 3/3] Fix as reviewed and add test --- .../card_locker/BarcodeSelectorActivity.java | 2 +- .../card_locker/LoyaltyCardEditActivity.java | 7 +- .../BarcodeSelectorActivityTest.java | 5 +- .../LoyaltyCardViewActivityTest.java | 66 +++++++++++++++++-- 4 files changed, 69 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java b/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java index 60ca02386..18becdcc6 100644 --- a/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java +++ b/app/src/main/java/protect/card_locker/BarcodeSelectorActivity.java @@ -145,7 +145,7 @@ public class BarcodeSelectorActivity extends AppCompatActivity public void onClick(View view) { Log.d(TAG, "Selected no barcode"); Intent result = new Intent(); - result.putExtra(BARCODE_FORMAT, LoyaltyCardEditActivity.NO_BARCODE); + result.putExtra(BARCODE_FORMAT, ""); result.putExtra(BARCODE_CONTENTS, cardId); BarcodeSelectorActivity.this.setResult(RESULT_OK, result); finish(); diff --git a/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java b/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java index 1ac8bb02d..3a45918c4 100644 --- a/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java +++ b/app/src/main/java/protect/card_locker/LoyaltyCardEditActivity.java @@ -37,7 +37,7 @@ public class LoyaltyCardEditActivity extends AppCompatActivity private static final String TAG = "CardLocker"; protected static final String NO_BARCODE = "_NO_BARCODE_"; - private static final int SELECT_BARCODE_REQUEST = 1; + protected static final int SELECT_BARCODE_REQUEST = 1; EditText storeFieldEdit; EditText noteFieldEdit; @@ -499,7 +499,7 @@ public class LoyaltyCardEditActivity extends AppCompatActivity } if(contents != null && contents.isEmpty() == false && - format != null && format.isEmpty() == false) + format != null) { Log.i(TAG, "Read barcode id: " + contents); Log.i(TAG, "Read format: " + format); @@ -508,7 +508,8 @@ public class LoyaltyCardEditActivity extends AppCompatActivity cardIdView.setText(contents); final TextView barcodeTypeField = findViewById(R.id.barcodeType); - barcodeTypeField.setText(format); + // Set special NO_BARCODE value to prevent onResume from overwriting it + barcodeTypeField.setText(format.isEmpty() ? LoyaltyCardEditActivity.NO_BARCODE : format); onResume(); } } diff --git a/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java b/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java index e03cc216f..993355c56 100644 --- a/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java +++ b/app/src/test/java/protect/card_locker/BarcodeSelectorActivityTest.java @@ -64,9 +64,8 @@ public class BarcodeSelectorActivityTest { activity.findViewById(R.id.noBarcode).performClick(); Intent resultIntent = shadowOf(activity).getResultIntent(); - // The BarcodeSelectorActivity should return the special NO_BARCODE string to differentiate - // from nothing being set yet - assertEquals(LoyaltyCardEditActivity.NO_BARCODE, resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_FORMAT)); + // The BarcodeSelectorActivity should return an empty string + assertEquals("", resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_FORMAT)); assertEquals("abcdefg", resultIntent.getStringExtra(BarcodeSelectorActivity.BARCODE_CONTENTS)); } diff --git a/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java b/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java index 0a38c4452..e139de488 100644 --- a/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java +++ b/app/src/test/java/protect/card_locker/LoyaltyCardViewActivityTest.java @@ -164,10 +164,10 @@ public class LoyaltyCardViewActivityTest assertNotNull(bundle); Intent resultIntent = new Intent(intent); - Bundle resultBuddle = new Bundle(); - resultBuddle.putString(Intents.Scan.RESULT, BARCODE_DATA); - resultBuddle.putString(Intents.Scan.RESULT_FORMAT, BARCODE_TYPE); - resultIntent.putExtras(resultBuddle); + Bundle resultBundle = new Bundle(); + resultBundle.putString(Intents.Scan.RESULT, BARCODE_DATA); + resultBundle.putString(Intents.Scan.RESULT_FORMAT, BARCODE_TYPE); + resultIntent.putExtras(resultBundle); // Respond to image capture, success shadowOf(activity).receiveResult( @@ -176,6 +176,38 @@ public class LoyaltyCardViewActivityTest resultIntent); } + /** + * Initiate and complete a barcode selection, either in success + * or in failure + */ + private void selectBarcodeWithResult(final Activity activity, final int buttonId, final String barcodeData, final String barcodeType, final boolean success) throws IOException + { + // Start image capture + final Button captureButton = activity.findViewById(buttonId); + captureButton.performClick(); + + ShadowActivity.IntentForResult intentForResult = shadowOf(activity).peekNextStartedActivityForResult(); + assertNotNull(intentForResult); + + Intent intent = intentForResult.intent; + assertNotNull(intent); + + Bundle bundle = intent.getExtras(); + assertNotNull(bundle); + + Intent resultIntent = new Intent(intent); + Bundle resultBundle = new Bundle(); + resultBundle.putString(BarcodeSelectorActivity.BARCODE_FORMAT, barcodeType); + resultBundle.putString(BarcodeSelectorActivity.BARCODE_CONTENTS, barcodeData); + resultIntent.putExtras(resultBundle); + + // Respond to barcode selection, success + shadowOf(activity).receiveResult( + intent, + success ? Activity.RESULT_OK : Activity.RESULT_CANCELED, + resultIntent); + } + private void checkFieldProperties(final Activity activity, final int id, final int visibility, final String contents) { @@ -551,6 +583,32 @@ public class LoyaltyCardViewActivityTest saveLoyaltyCardWithArguments(activity, "store", "note", BARCODE_DATA, LoyaltyCardEditActivity.NO_BARCODE, false); } + @Test + public void removeBarcodeFromLoyaltyCard() throws IOException + { + ActivityController activityController = createActivityWithLoyaltyCard(true); + Activity activity = (Activity)activityController.get(); + DBHelper db = new DBHelper(activity); + + db.insertLoyaltyCard("store", "note", BARCODE_DATA, BARCODE_TYPE, Color.BLACK, Color.WHITE); + + activityController.start(); + activityController.visible(); + activityController.resume(); + + // First check if the card is as expected + checkAllFields(activity, ViewMode.UPDATE_CARD, "store", "note", BARCODE_DATA, BARCODE_TYPE); + + // Complete empty barcode selection successfully + selectBarcodeWithResult(activity, R.id.enterButton, BARCODE_DATA, "", true); + + // Check if the barcode type is NO_BARCODE as expected + checkAllFields(activity, ViewMode.UPDATE_CARD, "store", "note", BARCODE_DATA, LoyaltyCardEditActivity.NO_BARCODE); + + // Check if the special NO_BARCODE string doesn't get saved + saveLoyaltyCardWithArguments(activity, "store", "note", BARCODE_DATA, LoyaltyCardEditActivity.NO_BARCODE, false); + } + @Test public void startCheckFontSizes() {