Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Kamila/add friends button #196

Merged
merged 26 commits into from
Apr 25, 2022
Merged

Kamila/add friends button #196

merged 26 commits into from
Apr 25, 2022

Conversation

kamilababayeva
Copy link
Collaborator

@kamilababayeva kamilababayeva commented Apr 22, 2022

This PR contains a first basic version of adding friends.

Changes:

  • "Add" button in user search to add user as a friend
  • Logic to store user as a friend of a current user in firebase

image

TODO:

  • change button interface when clicked to add
  • do not display "Add" button if user already a friend

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo found, otherwise good job 👍

@jiabaow jiabaow marked this pull request as ready for review April 24, 2022 13:42
@jiabaow jiabaow self-requested a review April 24, 2022 13:42
Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvements!
I added a few comments and one first issue which should not be forgot:
Each time the build fails on e: /tmp/cirrus-ci-build/app/src/androidTest/java/ch/sdp/vibester/activity/SearchUserActivityTest.kt: (20, 32): Unresolved reference: UserProfile
e: /tmp/cirrus-ci-build/app/src/androidTest/java/ch/sdp/vibester/activity/SearchUserActivityTest.kt: (21, 32): Unresolved reference: UserProfileAdapter

* @param searchInput search text inputed by user
* @param callback function to call with found users by username
*
* Comment about \uf8ff:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest putting this comment right below line 89 so that when people read the code, it's easier for them to find the explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

var ranking: Int = 0,
var uid: String = ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need an uid if the handle is already unique for each user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure whether the handle is correct. I saved the uid since the user is stored by it. We can discuss it next meeting.

val extract = File("storage/emulated/0/Download", "extract_of_$songName")
assert(!extract.exists())
}
// @Test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe explain in a comment why this test is commented

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@kamilababayeva
Copy link
Collaborator Author

Thanks for the improvements! I added a few comments and one first issue which should not be forgot: Each time the build fails on e: /tmp/cirrus-ci-build/app/src/androidTest/java/ch/sdp/vibester/activity/SearchUserActivityTest.kt: (20, 32): Unresolved reference: UserProfile e: /tmp/cirrus-ci-build/app/src/androidTest/java/ch/sdp/vibester/activity/SearchUserActivityTest.kt: (21, 32): Unresolved reference: UserProfileAdapter

thanks! fixed the issue with the build!

@codeclimate
Copy link

codeclimate bot commented Apr 24, 2022

Code Climate has analyzed commit e2c4361 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 75.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 86.3% (-1.2% change).

View more on Code Climate.

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice work !

Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kamilababayeva kamilababayeva merged commit ee7258c into main Apr 25, 2022
@kamilababayeva kamilababayeva deleted the kamila/add_friends_button branch April 25, 2022 05:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add logic to add user as a friend Add UI to add friends during the user search
3 participants