AC-742 refactoring code for better support for localization #723

LuGO0 · 2020-04-16T22:34:09Z

Description of what I changed

refactoring and extracting string resources for better support of localization in the client.
updated the ApplicationConstants

Issue I worked on

JIRA Issue: https://issues.openmrs.org/projects/AC/issues/AC-742

Checklist: I completed these to help reviewers :)

My pull request only contains ONE single commit
(the number above, next to the 'Commits' tab is 1).
I have added tests to cover my changes. (If you refactored
existing code that was well tested you do not have to add tests)
All new and existing tests passed.
My pull request is based on the latest changes of the master branch.

LuGO0 · 2020-04-16T22:35:36Z

@f4ww4z Sir please review the strings I have added to the resources file ,after which I will add the translations for them!!.

openmrs-client/schemas/org.openmrs.mobile.databases.AppDatabase/1.json

...s-client/src/main/java/org/openmrs/mobile/activities/dialog/CameraOrGalleryPickerDialog.java

...client/src/main/java/org/openmrs/mobile/activities/formdisplay/FormDisplayMainPresenter.java

openmrs-client/src/main/res/values/strings.xml

f4ww4z

@LuGO0 See my comments below.

openmrs-client/src/main/java/org/openmrs/mobile/activities/ACBaseActivity.java

...lient/src/main/java/org/openmrs/mobile/activities/addeditpatient/AddEditPatientActivity.java

openmrs-client/src/main/java/org/openmrs/mobile/services/ConceptDownloadService.java

...client/src/main/java/org/openmrs/mobile/utilities/ActiveAndroid/content/ContentProvider.java

openmrs-client/src/main/res/layout/fragment_login.xml

openmrs-client/src/main/res/values/strings.xml

LuGO0 · 2020-04-23T14:10:11Z

@f4ww4z can you please review again so that I can finally add translations to the added string resources!!

LuGO0 · 2020-05-07T08:19:12Z

@f4ww4z any updates on this ??

f4ww4z

@LuGO0 Sorry for the delay, see my comments below.

openmrs-client/src/main/java/org/openmrs/mobile/activities/dashboard/DashboardFragment.java

openmrs-client/src/main/java/org/openmrs/mobile/databases/entities/VisitEntity.java

openmrs-client/src/main/java/org/openmrs/mobile/databases/entities/ConceptEntity.java

openmrs-client/src/main/java/org/openmrs/mobile/services/ConceptDownloadService.java

LuGO0 · 2020-05-07T23:18:44Z

some translations were tough I had to use google Translate XD !!
@rishabh-997 please review the translations.
@f4ww4z please review the changes I will squash my commits .

rishabh-997 · 2020-05-08T15:47:33Z

some translations were tough I had to use google Translate XD !!
@rishabh-997 please review the translations.
@f4ww4z please review the changes I will squash my commits .

The Hindi translations are appropriate. Make sure to add JIRA ticket to the title and resolve the ci errors.

LuGO0 · 2020-05-09T23:06:27Z

merge conflicts resolved and the build works fine for me idk why the CI build fails

rishabh-997 · 2020-05-10T06:16:45Z

merge conflicts resolved and the build works fine for me idk why the CI build fails

You can click on CI details to see which particular test is failing and then run that test locally to check for exact error... You can run the entire presenter folder all together to see if all the tests are passing or not.

f4ww4z · 2020-05-10T15:16:53Z

@LuGO0 Please fix the merge conflicts and also take a look at these null pointer exceptions in the CI.

LuGO0 · 2020-05-10T15:27:40Z

@LuGO0 Please fix the merge conflicts and also take a look at these null pointer exceptions in the CI.

@f4ww4z I am looking into these errors !! and will resolve conflicts

LuGO0 · 2020-05-10T20:22:39Z

@f4ww4z I found out thatResources.getSystem() is null in the tests bt it works fine when app executes should I make changes in the testFile ? I will resolve conflicts once these tests pass !

f4ww4z · 2020-05-14T07:12:00Z

@f4ww4z I found out thatResources.getSystem() is null in the tests bt it works fine when app executes should I make changes in the testFile ? I will resolve conflicts once these tests pass !

@LuGO0 Yes you need to change test files. You can either use mocks to mock the getString part or create instrumentation tests.

LuGO0 · 2020-05-14T07:56:23Z

Okay @f4ww4z

LuGO0 · 2020-05-20T12:39:03Z

@rishabh-997 @f4ww4z I have removed all instances of Resources.getSystem() from all the presenters and instead delegated the views those are attached to it to display the strings.
but what should I do with the Repository class which uses ToastUtil.error to toast a lot of errors I couldnt find a work around for repository classes

LuGO0 · 2020-05-20T12:43:37Z

@rishabh-997 @f4ww4z I have removed all instances of Resources.getSystem() from all the presenters and instead delegated the views those are attached to it to display the strings.
but what should I do with the Repository class which uses ToastUtil.error to toast a lot of errors I couldnt find a work around for repository classes

the best I could think was a callback interface which would be implemented in the view and delegated down uptill the repository class or add a appContext to repository class.

LuGO0 · 2020-05-21T09:28:50Z

@rishabh-997 I coudnt test it on my phone how do we log in when demo server is down is there any way ??

codecov-commenter · 2020-05-21T09:37:48Z

Codecov Report

Merging #723 into master will decrease coverage by 0.06%.
The diff coverage is 17.78%.

@@            Coverage Diff             @@
##           master     #723      +/-   ##
==========================================
- Coverage   11.84%   11.77%   -0.07%     
==========================================
  Files         237      238       +1     
  Lines       10571    10641      +70     
  Branches     1018     1028      +10     
==========================================
+ Hits         1252     1253       +1     
- Misses       9237     9306      +69     
  Partials       82       82

Impacted Files	Coverage Δ
.../org/openmrs/mobile/activities/ACBaseActivity.java	`0.00% <0.00%> (ø)`
.../org/openmrs/mobile/activities/ACBaseFragment.java	`10.00% <ø> (ø)`
...a/org/openmrs/mobile/activities/BasePresenter.java	`55.55% <0.00%> (ø)`
.../activities/activevisits/ActiveVisitsActivity.java	`0.00% <0.00%> (ø)`
.../activities/activevisits/ActiveVisitsFragment.java	`0.00% <ø> (ø)`
...ivities/addeditpatient/AddEditPatientActivity.java	`0.00% <0.00%> (ø)`
...ivities/addeditpatient/AddEditPatientFragment.java	`0.00% <0.00%> (ø)`
...ditpatient/SimilarPatientsRecyclerViewAdapter.java	`0.00% <0.00%> (ø)`
...ctivities/community/contact/ContactUsActivity.java	`0.00% <0.00%> (ø)`
...ctivities/community/contact/ContactUsContract.java	`0.00% <ø> (ø)`
... and 180 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c8e3ca...7fe0821. Read the comment docs.

rishabh-997 · 2020-05-21T13:06:06Z

@rishabh-997 I coudnt test it on my phone how do we log in when demo server is down is there any way ??

You can use alternative server.

LuGO0 · 2020-05-21T13:29:38Z

@rishabh-997 I coudnt test it on my phone how do we log in when demo server is down is there any way ??

You can use alternative server.

thanks Rishabh Ill check the app now and then request review !!

LuGO0 · 2020-05-22T08:06:30Z

@f4ww4z @rishabh-997 I have run all the tests locally now it works fine and the app also runs fine as far as I have tested please review !!

...s-client/src/main/java/org/openmrs/mobile/activities/dialog/CameraOrGalleryPickerDialog.java

...client/src/main/java/org/openmrs/mobile/activities/formdisplay/FormDisplayMainPresenter.java

openmrs-client/src/main/java/org/openmrs/mobile/activities/login/LoginFragment.java

openmrs-client/src/main/java/org/openmrs/mobile/api/RestServiceBuilder.java

rishabh-997

I have reviewed the Activities package, kindly address the above comments. Also pull recent changes and resolve merge conflicts.
Also, make sure to edit only the required files next time so we can be specific to the files that have to reviewed...

LuGO0 · 2020-05-25T20:12:28Z

@rishabh-997 I was doing a code parsing for all the hard coded string and it did code reformatting I dont know how bt It was good so I let it that way.
I will take care of this from next time
Thanks for reviewing :)

LuGO0 · 2020-05-28T20:41:47Z

@f4ww4z @rishabh-997 please further review it .Its getting difficult to merge conflicts with this PR and keep it upto date with master 😅

f4ww4z

@LuGO0 Yes, fixing code formatting should be in another issue and now it's difficult to review your changes. Please address my comments and then we should be good to merge.

...client/src/main/java/org/openmrs/mobile/activities/formdisplay/FormDisplayMainPresenter.java

f4ww4z · 2020-05-30T04:03:27Z

openmrs-client/src/test/java/org/openmrs/mobile/utilities/DateUtilsTest.java

-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;


Don't change to star imports.

fixed this @f4ww4z Sir.

f4ww4z · 2020-05-30T04:04:02Z

openmrs-client/src/test/java/org/openmrs/mobile/utilities/DateUtilsTest.java

-        DateTime dateTime;
-        String nullDateAsString = null;
-        DateTimeFormatter formatter = DateTimeFormat
+    @RunWith(PowerMockRunner.class)


No need to reindent this.

done @f4ww4z Sir.

LuGO0 · 2020-05-30T08:55:29Z

@f4ww4z Sir I have made requested changes and will sqash changes once you approve of them!!

f4ww4z

LGTM @LuGO0 !

LuGO0 force-pushed the string_localization_refactoring branch from fe0b72d to c6b0f0e Compare April 16, 2020 22:56

LuGO0 commented Apr 16, 2020

View reviewed changes

openmrs-client/schemas/org.openmrs.mobile.databases.AppDatabase/1.json Outdated Show resolved Hide resolved

f4ww4z force-pushed the master branch from fc772c7 to 0fccb0c Compare April 17, 2020 07:16

LuGO0 force-pushed the string_localization_refactoring branch from c6b0f0e to ae3f78e Compare April 17, 2020 08:09

jawidMuhammadi reviewed Apr 21, 2020

View reviewed changes

...s-client/src/main/java/org/openmrs/mobile/activities/dialog/CameraOrGalleryPickerDialog.java Outdated Show resolved Hide resolved

jawidMuhammadi reviewed Apr 21, 2020

View reviewed changes

...client/src/main/java/org/openmrs/mobile/activities/formdisplay/FormDisplayMainPresenter.java Outdated Show resolved Hide resolved

jawidMuhammadi suggested changes Apr 21, 2020

View reviewed changes

openmrs-client/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

f4ww4z requested changes Apr 22, 2020

View reviewed changes

LuGO0 force-pushed the string_localization_refactoring branch 4 times, most recently from 623dedf to ad4e246 Compare April 23, 2020 14:08

f4ww4z reviewed May 7, 2020

View reviewed changes

LuGO0 force-pushed the string_localization_refactoring branch from 0c21bed to f527260 Compare May 7, 2020 23:16

LuGO0 marked this pull request as ready for review May 7, 2020 23:20

LuGO0 requested a review from f4ww4z May 7, 2020 23:22

LuGO0 changed the title ~~refactoring code for better support for localization~~ AC-742 refactoring code for better support for localization May 8, 2020

LuGO0 force-pushed the string_localization_refactoring branch from f527260 to 5ed26c7 Compare May 9, 2020 23:05

LuGO0 force-pushed the string_localization_refactoring branch from bdda201 to 67b0ee8 Compare May 21, 2020 09:25

LuGO0 force-pushed the string_localization_refactoring branch from 67b0ee8 to a2c0bd1 Compare May 22, 2020 08:04

LuGO0 requested a review from rishabh-997 May 25, 2020 06:46

rishabh-997 reviewed May 25, 2020

View reviewed changes

...s-client/src/main/java/org/openmrs/mobile/activities/dialog/CameraOrGalleryPickerDialog.java Outdated Show resolved Hide resolved

rishabh-997 reviewed May 25, 2020

View reviewed changes

...client/src/main/java/org/openmrs/mobile/activities/formdisplay/FormDisplayMainPresenter.java Show resolved Hide resolved

rishabh-997 reviewed May 25, 2020

View reviewed changes

openmrs-client/src/main/java/org/openmrs/mobile/activities/login/LoginFragment.java Outdated Show resolved Hide resolved

rishabh-997 reviewed May 25, 2020

View reviewed changes

openmrs-client/src/main/java/org/openmrs/mobile/api/RestServiceBuilder.java Outdated Show resolved Hide resolved

rishabh-997 requested changes May 25, 2020

View reviewed changes

LuGO0 force-pushed the string_localization_refactoring branch from a2c0bd1 to 177cb49 Compare May 26, 2020 04:29

LuGO0 requested a review from rishabh-997 May 26, 2020 18:50

f4ww4z requested changes May 30, 2020

View reviewed changes

LuGO0 added 4 commits May 30, 2020 13:28

AC-742 refactored codebase for extension of localization

8261d2a

AC-742 added translations and updated application constants

15678f8

fixing all the reviews on activities package

219e591

Update DateUtilsTest.java

7fe0821

LuGO0 force-pushed the string_localization_refactoring branch from 4d36a9f to 7fe0821 Compare May 30, 2020 08:20

LuGO0 requested a review from f4ww4z May 31, 2020 09:08

f4ww4z approved these changes Jun 1, 2020

View reviewed changes

f4ww4z merged commit fba7fe2 into openmrs:master Jun 1, 2020

f4ww4z mentioned this pull request Jun 1, 2020

AC-764 Updated Contact Screen UI #746

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AC-742 refactoring code for better support for localization #723

AC-742 refactoring code for better support for localization #723

LuGO0 commented Apr 16, 2020 •

edited

Loading

LuGO0 commented Apr 16, 2020 •

edited

Loading

f4ww4z left a comment

LuGO0 commented Apr 23, 2020 •

edited

Loading

LuGO0 commented May 7, 2020

f4ww4z left a comment

LuGO0 commented May 7, 2020

rishabh-997 commented May 8, 2020

LuGO0 commented May 9, 2020

rishabh-997 commented May 10, 2020

f4ww4z commented May 10, 2020

LuGO0 commented May 10, 2020

LuGO0 commented May 10, 2020 •

edited

Loading

f4ww4z commented May 14, 2020

LuGO0 commented May 14, 2020

LuGO0 commented May 20, 2020

LuGO0 commented May 20, 2020 •

edited

Loading

LuGO0 commented May 21, 2020

codecov-commenter commented May 21, 2020 •

edited

Loading

rishabh-997 commented May 21, 2020

LuGO0 commented May 21, 2020 •

edited

Loading

LuGO0 commented May 22, 2020 •

edited

Loading

rishabh-997 left a comment

LuGO0 commented May 25, 2020 •

edited

Loading

LuGO0 commented May 28, 2020

f4ww4z left a comment

f4ww4z May 30, 2020

LuGO0 May 30, 2020

f4ww4z May 30, 2020

LuGO0 May 30, 2020

LuGO0 commented May 30, 2020

f4ww4z left a comment

AC-742 refactoring code for better support for localization #723

AC-742 refactoring code for better support for localization #723

Conversation

LuGO0 commented Apr 16, 2020 • edited Loading

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

LuGO0 commented Apr 16, 2020 • edited Loading

f4ww4z left a comment

Choose a reason for hiding this comment

LuGO0 commented Apr 23, 2020 • edited Loading

LuGO0 commented May 7, 2020

f4ww4z left a comment

Choose a reason for hiding this comment

LuGO0 commented May 7, 2020

rishabh-997 commented May 8, 2020

LuGO0 commented May 9, 2020

rishabh-997 commented May 10, 2020

f4ww4z commented May 10, 2020

LuGO0 commented May 10, 2020

LuGO0 commented May 10, 2020 • edited Loading

f4ww4z commented May 14, 2020

LuGO0 commented May 14, 2020

LuGO0 commented May 20, 2020

LuGO0 commented May 20, 2020 • edited Loading

LuGO0 commented May 21, 2020

codecov-commenter commented May 21, 2020 • edited Loading

Codecov Report

rishabh-997 commented May 21, 2020

LuGO0 commented May 21, 2020 • edited Loading

LuGO0 commented May 22, 2020 • edited Loading

rishabh-997 left a comment

Choose a reason for hiding this comment

LuGO0 commented May 25, 2020 • edited Loading

LuGO0 commented May 28, 2020

f4ww4z left a comment

Choose a reason for hiding this comment

f4ww4z May 30, 2020

Choose a reason for hiding this comment

LuGO0 May 30, 2020

Choose a reason for hiding this comment

f4ww4z May 30, 2020

Choose a reason for hiding this comment

LuGO0 May 30, 2020

Choose a reason for hiding this comment

LuGO0 commented May 30, 2020

f4ww4z left a comment

Choose a reason for hiding this comment

LuGO0 commented Apr 16, 2020 •

edited

Loading

LuGO0 commented Apr 16, 2020 •

edited

Loading

LuGO0 commented Apr 23, 2020 •

edited

Loading

LuGO0 commented May 10, 2020 •

edited

Loading

LuGO0 commented May 20, 2020 •

edited

Loading

codecov-commenter commented May 21, 2020 •

edited

Loading

LuGO0 commented May 21, 2020 •

edited

Loading

LuGO0 commented May 22, 2020 •

edited

Loading

LuGO0 commented May 25, 2020 •

edited

Loading