Fixes Runtime Permissions

[abs51295]

Fixes #397

GIF of the fixed issue:

[sandarumk]

Can you add a gif of the fixed one.

[chamika]

I think this prompts permission dialog when the application starts. The proper behavior would be prompting when the user tries to open Circle of Trust.
@sandarumk Is there any other part of the application which needs location permission apart from Circle of Trust?

[sandarumk]

No. It is the only place which uses location from the device. Get Help Now is location specific. But the user has to choose the location manually.

[abs51295]

@chamika @sandarumk Yes Circle of Trust is the only part which needs location permission. But there are also other fragments in the application which need dangerous permissions like SEND_SMS, CALL_PHONE and READ_CONTACTS. So I have requested all of them in the code of the Main Activity itself. So should I separate them in respective fragments?

[sandarumk]

It is a good practice to rebase rather than merge (of course there has been lot of discussion going on and different people have different views. But in this context we prefer rebasing because it provides more cleaner code). since merge will have merge commits. To remove unnecessary commits from your PR consider squashing next time.

[chamika]

@abs51295 Can you list down the fragment and their permission if you are going to separate them?
Ex: TrusteesFragment -> READ_CONTACTS,...

[abs51295]

@sandarumk For sure I will keep in mind from next time 👍 .
@chamika Here is the list of activities or fragments which need the dangerous permissions as discussed:

Trustees.java -> READ_CONTACTS
OtherStaffContent.java -> CALL_PHONE and SEND_SMS
ContactPostStaff.java -> CALL_PHONE and SEND_SMS
CircleOfTrustFragment.java -> ACCESS_FINE_LOCATION ,SEND_SMS, READ_CONTACTS(via ContactPhotoLoader.java)

[chamika]

A user can navigate to Trustees only through Circle of Trusts fragment. So showing READ_CONTACTS runtime permission in CircleOfTrustFragment will remove Trustees from the list. I think it will not be hard to request the permissions individually in fragments instead of application start since there are only 3 fragments.
@sandarumk WDYT?

[sandarumk]

agree with you @chamika

[abs51295]

Hey @chamika @sandarumk I am sorry I am busy with exams. I will update this asap as suggested by @chamika .

[HimanshuS01]

Hey @abs51295 as this is a single issue , so one commit per feature is generally recommended by the mentors. Please squash the commits into one that will make easy to track down the change(s) per issue/feature independently. :-)

[abs51295]

Hey Himanshu! Thanks for the hint. But the commits can be squashed into one while merging also right?

[HimanshuS01]

yeah @abs51295 :-) . But we have to strictly follow the PR best practices guidelines. For reference please refer to the commit section in this link .

[abs51295]

Yeah but I think have messed up this time, unfortunately. I used merge instead of rebase and now I am facing problems to squash commits into one. I would request @sandarumk @chamika to allow it this time. I would surely follow the rebase concept next time.

[sandarumk]

1. Please resolve the conflicts.
2. Learning to fix a git messup is also a part of open source contribution.

[abs51295]

@sandarumk Please review.