Fix #285 #295
Fix #285 #295
Conversation
JoaquimLey
commented
Sep 10, 2016
•
JoaquimLey
commented
- Fix Reading note exporting
- Abstract to a pseudo MVP architecture (ExportPresenter/ExportPresenterView)
- Improvements on export algorithm
- Simple unit tests
- Update gradle version
|
|
||
| if (all) { | ||
| readings = dB.getGlucoseReadings(); | ||
| public ExportPresenter(Activity activity) { |
There was a problem hiding this comment.
Why not to have parameter for constructor only ExportView type? Why do we need to cast it?
There was a problem hiding this comment.
Because you need the Activity since you are handling the permissions in the presenter (it wouldn't be really my choice but see line 140) which requires to have an activity, the cast is to ensure said activity is implementing the required View interface.
There was a problem hiding this comment.
Clear! It was there already. As for me, a presenter should not have any dependency to Android API, but this is the discussion for the android team.
As the quick solution I would have two parameters - one view and one activity. And remove casting!
There was a problem hiding this comment.
Yes it would be easy to abstract the Android apis from the Presenter if there was no Permission handling there (and it shouldn't).
With the current setup you'll be instantiating something like this:
new ExportPresenter(this, this); not pretty, but refactoring this wouldn't make the PR with to many differences.
|
@JoaquimLey, thank you for the contribution! I reviewed and added some comments. Can you reply? Can you also elaborate how the algorithm was improved? |
|
The current algorithm was querying the database on the UI thread, ensure there were |
…s, added todo to tests
|
@JoaquimLey thank you for saving the UI thread! This is really important. I'm not bug expert of the Realm, but looks like many references to Realm shouldn't be a problem (http://stackoverflow.com/questions/27777948/how-to-correctly-use-realm). Can you reproduce the issue? Can you debug and confirm cause with notes exporting? |
|
There were times where the notes would export and others not, I couldn't find a pattern but I believe this issue was more related to the query of the note, not the saving or writing to the CSV file. Once again I thought this was outside of this issue scope and didn't fix. I loosely assumed this could be the problem, having concurrent data access, when I changed it, it worked. |
|
@paolorotolo, if you follow the conversation, what do you think? |
|
Hello again, so you guys don't what the contribution(s)? |
|
Actually completely opposite - we love contributions! |
|
Hey, sorry I totally missed this pr. The code looks good, can you please fix the conflict on |
…s, added todo to tests
|
@paolorotolo fixed (squash to prevent commit verbosity). |
|
Merged, thanks. |
|
Great work thanks! |
|
Happy to help! ;) |
* Start working on Reminders. * Updated build gradle to 2.2.0-rc1 from beta * Add Reminders in Realm. Add migration to schema 4. * Add ArrayAdapter for reminders. * Continue work on Reminders. * Add method to get all active reminders * Automatic translation import (build 924). * Continue work on Reminders. * Fix migration. * Add reminders notification. * Fix some issues. * Codacy warnings fixed * Automatic translation import (build 937). * Fix reminder triggered on boot, update Gradle version to stable. * Update dependencies. * Fix #285 (#295) * Start working on Reminders. * Add Reminders in Realm. Add migration to schema 4. * Add ArrayAdapter for reminders. * Continue work on Reminders. * Add method to get all active reminders * Fix notes export, abstract to a pseudo MVP architecture * Continue work on Reminders. * Simple note tests * Fix notes export, abstract to a pseudo MVP architecture * Simple note tests * Improve quality from codacy report * Fix migration. * Class ReadingToCSV is not final, removed the redundant Sys.currentTMls, added todo to tests * Add reminders notification. * Improvements #2 on PR comments * Fix some issues. * Codacy warnings fixed * Automatic translation import (build 937). * Fix reminder triggered on boot, update Gradle version to stable. * Update dependencies. * Fix notes export, abstract to a pseudo MVP architecture * Simple note tests * Improve quality from codacy report * Class ReadingToCSV is not final, removed the redundant Sys.currentTMls, added todo to tests * Improvements #2 on PR comments * Add labels on reminders. * Add label on reminder notification. * Updated deprecated comment from WebviewClient * Moved URLs as fields * Extracted some methods and leaved the code cleaner * Created basic network connectivity * Extracted external glucosio links to separate class * Create and improved the basic network availability implementation * Refactored the LicenseActivity: - Made a presenter - Some basic interaction tests * Applied some format * Renamed to ExternalLinkActivity * Dialog made not cancelable * Typo fix for preferences terms * Initialized map with predefined 3 elements * Created launcher methodˆ * Replaced old call to Terms * Replaced old call to Terms on HelloActivity * Replaced all old called with the new launch method * Created extractor method for extracting title and url from intent extra * Removed obsolete tests and changed behaviour for presenter * Added new strings. Fix label layout. * Automatic translation import (build 957). * - Added annotations for non null parameters - Replaced empty string checks with Android classes - Private method for extra extracting * Fixed message for evening case * Removed unnecessary local variable * Allow comma as decimal separator when adding Glucose reading * Added contributor credits * Set locale back to default after tests in ReadingToolsTest * Refactor AddReadingPresenter * Cleaning some codacy useful warnings (#310) * Removing unused variables and fields, inlining fields used once, minor improvements * Removed unused misleading badge * Remove categories creaition from assistant (fix #245) (#308) * Removed unused functionality, introduced butterknife, removed categories mentioning and first tests for fragment * Removed unused field and minor cleanup * Removed cardview since it is not appropriate here * Removed whitespace * Added test for adapter * Simplified adapter instantiation * Added test for presenter * Add mmol/L unit check in validGlucose method * Remove hardcoded strings for concentrations like mg/dL * Add number formatting for values when converting to string * Improve FreeStyle Libre support (#306) * Updated build gradle to 2.2.0-rc1 from beta * Add labels on reminders. * Add label on reminder notification. * Updated deprecated comment from WebviewClient * Moved URLs as fields * Extracted some methods and leaved the code cleaner * Created basic network connectivity * Extracted external glucosio links to separate class * Create and improved the basic network availability implementation * Refactored the LicenseActivity: - Made a presenter - Some basic interaction tests * Applied some format * Renamed to ExternalLinkActivity * Dialog made not cancelable * Typo fix for preferences terms * Initialized map with predefined 3 elements * Created launcher methodˆ * Replaced old call to Terms * Replaced old call to Terms on HelloActivity * Replaced all old called with the new launch method * Created extractor method for extracting title and url from intent extra * Removed obsolete tests and changed behaviour for presenter * FreestyleLibre: divide raw value by 8.5 xdrip+/LimiTTer are dividing by 8.5, seems to work good for many people there, including me * FreesyleLibre: use latest value We wanted to use the latest history value, but used the oldest one, because array is filled from newest to oldest Also we should use trend values instead of history values, as the newest history value can be up to 15 minutes old * Added new strings. Fix label layout. * Automatic translation import (build 957). * - Added annotations for non null parameters - Replaced empty string checks with Android classes - Private method for extra extracting * Round the glucose value before converting to int * Fixed message for evening case * Removed unnecessary local variable * Allow comma as decimal separator when adding Glucose reading * Added contributor credits * Set locale back to default after tests in ReadingToolsTest * Refactor AddReadingPresenter * Cleaning some codacy useful warnings (#310) * Removing unused variables and fields, inlining fields used once, minor improvements * Removed unused misleading badge * Remove categories creaition from assistant (fix #245) (#308) * Removed unused functionality, introduced butterknife, removed categories mentioning and first tests for fragment * Removed unused field and minor cleanup * Removed cardview since it is not appropriate here * Removed whitespace * Added test for adapter * Simplified adapter instantiation * Added test for presenter * Fix mmol edit bug. * Fix A1C conversion bug. * Support 'Almost' translated languages This application is used in various countries. And in Crowdin, some languages are already 100% translated or almost translated. But Glucosio is still support only English. These features are enhancement of it. And also, you can find Crowdin's API for translation status. But there are too many non-translated languages and it was useless to get it's all data. The API needs Auth key so until it is cleared, just maintaining available strings manually would be better. I already made code, but I will just keep it. - Add available_languages.xml of 'almost' translated languages - Add updateLocale code on MainPresenter when there is already user profile in DB - Swap two codes order of calling MainPresenter for update MainActivities in start - Add localeHelper variable for MainActivity - Changed getLocalesWithTranslation to available languages - Fixed bug in build.gradle. It should contain one more backslash * Fix code duplication. * Target API 25. Add App Shortcuts. * Fix App Shortcuts. * Releasing Glucosio 1.3.0 * Fixed for StringIndexOutOfBoundsException * Trying to fix travis * Releasing Glucosio 1.3.0 (38)
