Landry333 / Big-Owl

soen 490 project
2 stars 2 forks source link

#273 bug fix contacts black screen #276

Closed refatK closed 3 years ago

refatK commented 3 years ago

Related Issues

Closes #273

Description

Contacts require a database call to get in Android, so I followed Along with Android's explanation of how to get a Contacts list and used a LoaderManager to get the contacts, which gets them asyncly. Furthermore, I added a max of 50 results upon contacts loading, to get more specific result, the user can search the first name, last name, or number of the contacts.

Other improvements added was the use of the number formatter, which will make the results filter out numbers that cant exist. I also added the progress bar when contacts are loading.

NOTE: There are 2 things I'm unsure of and would like some feed back

How to check PR:

  1. Add many contacts to the Android device if you can
  2. Run the app and login as any user if not already logged in
  3. Click "Add Users"
  4. Click "Invite from Contact List" (accept permissions if requested)

You should see your contacts as the app always showed. Please also test the search bar.

codecov-io commented 3 years ago

Codecov Report

Merging #276 (4b6277a) into staging (a492474) will increase coverage by 0.11%. The diff coverage is 16.85%.

Impacted file tree graph

@@              Coverage Diff              @@
##             staging     #276      +/-   ##
=============================================
+ Coverage      42.63%   42.75%   +0.11%     
  Complexity         1        1              
=============================================
  Files             89       89              
  Lines           2988     2994       +6     
  Branches         167      161       -6     
=============================================
+ Hits            1274     1280       +6     
- Misses          1669     1670       +1     
+ Partials          45       44       -1     
Impacted Files Coverage Δ Complexity Δ
...pp/activity/FingerprintAuthenticationActivity.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../example/bigowlapp/activity/LoginPageActivity.java 54.54% <0.00%> (ø) 0.00 <0.00> (ø)
...mple/bigowlapp/activity/SearchContactsByPhone.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../bigowlapp/activity/SearchContactsToSupervise.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...pp/activity/SendingRequestToSuperviseActivity.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...le/bigowlapp/utils/AuthenticatorByPhoneNumber.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...example/bigowlapp/utils/SupervisorSmsListener.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...example/bigowlapp/activity/SignUpPageActivity.java 90.76% <50.00%> (ø) 0.00 <0.00> (ø)
...lapp/repository/exception/EmptyFieldException.java 50.00% <50.00%> (ø) 0.00 <0.00> (ø)
...xample/bigowlapp/activity/EditProfileActivity.java 92.95% <100.00%> (+0.85%) 0.00 <0.00> (ø)
... and 1 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 a492474...4b6277a. Read the comment docs.

refatK commented 3 years ago

I tested the feature and it works fine as expected. As concerning your question, yes 50 users is a good number as the user will quickly realized that not all users from phone contacts were displayed in case of high number of contacts .You can leave the loading bar, it is appearing in case of high number of contacts like in my case. I only had a problem where the app doesn't go to the SendingRequestToSuperviseActivity screen in case of existing user. But I guess as this wasn't part of your issue, we can check that problem in another issue. As for the code, it looks good to me so I will approve. Good job

Might have accidentally broke the existing number case. I'll check that.

ShawnGuerra-Bautista commented 3 years ago

I don't have this bug on my phone, but the feature works normally.

I checked the code; it's pretty clean. I see no problem; approved.

Landry333 commented 3 years ago

I know this is a bit aside of what your did here, but could you also add a number formatter to the input phone number when user is searching contact by typing the contact phone number. You can make it as another BUG issue if you like

refatK commented 3 years ago

I tested the feature and it works fine as expected. As concerning your question, yes 50 users is a good number as the user will quickly realized that not all users from phone contacts were displayed in case of high number of contacts .You can leave the loading bar, it is appearing in case of high number of contacts like in my case. I only had a problem where the app doesn't go to the SendingRequestToSuperviseActivity screen in case of existing user. But I guess as this wasn't part of your issue, we can check that problem in another issue. As for the code, it looks good to me so I will approve. Good job

@Landry333 You were right. I thought I fixed the issue but I had made a mistake. Please check again ad confirm there are no issues

refatK commented 3 years ago

I know this is a bit aside of what your did here, but could you also add a number formatter to the input phone number when user is searching contact by typing the contact phone number. You can make it as another BUG issue if you like

did it

Landry333 commented 3 years ago

I tested the fix you made. It is working fine, to make a request and also the number formatter to find a contact by typing a number. I know it was some kind of aside to the main thing you were doing, so good job!

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

22.5% 22.5% Coverage
0.8% 0.8% Duplication