Add additional JSON params for server control of update prompts #26
Conversation
|
@saraht129 Thanks! A couple of things:
If you get a chance to fix, that would be great, otherwise, I'll take care of it when I review. Thanks so much for the PR! |
|
Apologies for the build error. I've taken care of that and added instructions regarding the new parameters. |
|
Hi, any chance that this can be merged? Thanks! |
|
Just like to check if there's any update on this? |
|
@saraht129 Hi, sorry about the delay! I must have looked at the git notification on the phone but then forgot. I've added this to my list for tomorrow. Thanks! |
|
@Apisov Can you please take a look at this and especially confirm that the new switch code does the same as the replaced if/else chain (taking into account the new functionality)? Thanks! |
| @@ -231,13 +245,21 @@ private int checkVersionDigit(String[] minVersionNumbers, String[] currentVersio | |||
| private boolean checkVersionCode(JSONObject appJson) throws JSONException{ | |||
| if (!appJson.isNull(Constants.JSON_MIN_VERSION_CODE)) { | |||
| int minAppVersionCode = appJson.getInt(Constants.JSON_MIN_VERSION_CODE); | |||
| // If no config found, assume version check is enabled | |||
| Boolean versionCheckEnabled = appJson.has(Constants.JSON_ENABLE_VERSION_CHECK) ? appJson.getBoolean(Constants.JSON_ENABLE_VERSION_CHECK) : true; | |||
There was a problem hiding this comment.
As a small improvement, we can move the logic of getting Constants.JSON_ENABLE_VERSION_CHECK value to separate method that returns boolean.
Because we have duplication of the logic in the code.
| } | ||
|
|
||
| // If no config found, assume force update = false | ||
| Boolean forceUpdateEnabled = appJson.has(Constants.JSON_FORCE_ALERT_TYPE) ? appJson.getBoolean(Constants.JSON_FORCE_ALERT_TYPE) : false; |
There was a problem hiding this comment.
I think here it's possible to do the same as with Constants.JSON_ENABLE_VERSION_CHECK logic. But for Constants.JSON_FORCE_ALERT_TYPE
| alertType = SirenAlertType.FORCE; | ||
| } else { | ||
| switch (index) { | ||
| case 0: alertType = majorUpdateAlertType; break; |
There was a problem hiding this comment.
Maybe it will add more readability if we will use constants instead of "magic numbers".
ie INDEX_MAJOR = 0 ... INDEX_REVISION = 3
mikemee
changed the title
|
@saraht129 Released as version 1.5.0. Thanks for your the idea, your contribution and the follow-up! |
Close #25
This PR adds two additional parameters on the JSON file