Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redo networking because of backend changes #137

Merged
merged 6 commits into from
Mar 12, 2021
Merged

Conversation

lucyxubroad
Copy link
Contributor

Overview

Backend changed up their backend models to make the code more stable cuappdev/pear-backend#49 . In this PR, I change our models to fit their new model.

Changes Made

  • Basically added more GET routes since those got separated out from our user route and changed code throughout app to reflect that.

Test Coverage

Next Steps (delete if not applicable)

Related PRs or Issues (delete if not applicable)

#134

Screenshots (delete if not applicable)

Screen Shot Name

let userGoogleId = user.googleID {
profileImageView.kf.setImage(with: Base64ImageDataProvider(base64String: profilePictureURL, cacheKey: userGoogleId))
}
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm commenting this out for now because our server has some outdated users with the wrong profile photo string, which causes app to crash. Will comment back in when backend does this.

.socialMediaLinks(instagram: user.instagram, facebook: user.facebook)
.normalFont(".")
}
// if user.instagram != nil || user.facebook != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment out for now because backend took social media out of user model but I'm requesting they add it back. Will uncomment when that is done.

.socialMediaLinks(instagram: user.instagram, facebook: user.facebook)
.normalFont(".")
}
// if user.instagram != nil || user.facebook != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment out for now because backend took social media out of user model but I'm requesting they add it back. Will uncomment when that is done.

Copy link

@danielVebman danielVebman left a comment

Choose a reason for hiding this comment

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

Looks great 🤩

self.setUserAndTabPage(newUser: newUser)
self.getUserMatchThen(netId: newUser.netID) { [weak self] matches in
guard let self = self else { return }
if self.user == nil || self.user != newUser {

Choose a reason for hiding this comment

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

I haven't checked, but just be sure that == is defined as you expect for the user object.

self.profileImageView.kf.setImage(with: Base64ImageDataProvider(base64String: newUser.profilePictureURL, cacheKey: newUser.googleID))
let firstActiveMatch = newUser.matches.filter({ $0.status != "inactive" }).first
self.setupTabPageViewController(with: firstActiveMatch, user: newUser)
private func setUserAndTabPage(newUser: User, match: Match? = nil ) {

Choose a reason for hiding this comment

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

Delete space before ).

guard result.success else {
print("Network error: could not get user's pair.")
return
}
closure(result.data)

Choose a reason for hiding this comment

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

I would call it something more descriptive (and consistent with other places), like completion

@@ -19,7 +19,7 @@ struct CommunityUser: Codable {
let lastName: String?
let major: String?
let netID: String?
let profilePictureURL: String?
// let profilePictureURL: String?

Choose a reason for hiding this comment

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

Delete?

let lastName: String
let major: String
let netID: String
let hometown: String

Choose a reason for hiding this comment

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

alphabetize

Comment on lines +147 to +149
guard response.success else {
print("Network error: could not get user match.")
return
Copy link
Member

Choose a reason for hiding this comment

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

When a success doesn't even work :(

Comment on lines 24 to 26
static func == (lhs: User, rhs: User) -> Bool {
lhs.firstName == rhs.firstName &&
lhs.goals == rhs.goals &&
lhs.googleID == rhs.googleID &&
lhs.graduationYear == rhs.graduationYear &&
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if you enforce any invariants on the backend, but this could probably just be lhs.netID == rhs.netID since a user should not be able to sign up twice w/ the same ID (something would be broken then). Just a thought, but feels like cheating.

Copy link
Contributor

@amyqxhuang amyqxhuang left a comment

Choose a reason for hiding this comment

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

looks good!! 😊

}
}

private func getUserAvailabilitiesThen(_ closure: @escaping ([DaySchedule]) -> Void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

to keep it uniform, call this completion instead of closure?

func getMatchHistory(netID: String) -> Future<Response<[Match]>> {
networking(Endpoint.getMatchHistory(netID: netID)).decode()
}

// MARK: - Stored
Copy link
Contributor

Choose a reason for hiding this comment

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

what does Stored mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just temporarily hard coded stored values

@lucyxubroad lucyxubroad merged commit 013bd27 into master Mar 12, 2021
@lucyxubroad lucyxubroad deleted the lucy/redo-networking branch May 15, 2021 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants