Skip to content

users can now add and drop connections with other users#657

Open
ccorkery97 wants to merge 14 commits intodevelopfrom
API-Connections-update
Open

users can now add and drop connections with other users#657
ccorkery97 wants to merge 14 commits intodevelopfrom
API-Connections-update

Conversation

@ccorkery97
Copy link
Copy Markdown

  • Related Issue (include '#'):
    Issue As a user I would like to formally "connect" with another user #457
  • Description of changes made:
    Changes were done in Directory page where a user can now see their connections, see other users they are not connected to, add those users to their connection list, and drop users from their connection list.
  • Is the feature complete/bug resolved/etc..:
    the feature is complete
  • Any known bugs/strange behavior:
    no known bugs
  • Is there specific feedback you would like on these changes:
    some feedback on the design of the directory page would be useful because I wasn't sure where to show the users connections.
  • Screenshot(s):

@ccorkery97 ccorkery97 requested review from 0xZakk and Underwaterr July 8, 2020 14:29
Copy link
Copy Markdown
Contributor

@0xZakk 0xZakk left a comment

Choose a reason for hiding this comment

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

Are you missing some code here?

@ccorkery97 ccorkery97 requested a review from 0xZakk July 13, 2020 20:05
Copy link
Copy Markdown
Contributor

@0xZakk 0xZakk left a comment

Choose a reason for hiding this comment

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

@ccorkery97 This looks pretty good and is a good start. See my comments below.

Generally speaking, I'd say this is more complicated than it needs to be. Most of the logic you have here should be happening on the backend and you shouldn't need this much logic.

For example, to create a friend request, the signed in user should click a button that sends a request to the backend with the ID for the user who is the target of the friend request (probably just in the URL parameters). The backend should take that ID and create the request between the user with that ID and the currently signed in User. You can have a status field on it (like accepted) which defaults to false. When that other user signs in, there should be a separate page where they can see all their friend requests (i.e. requests where accepted is false) and then submit a request to approve or reject the request.

Comment thread api/.vscode/settings.json
Comment thread api/accounts/views.py
Comment thread client/src/App.js Outdated
Comment thread client/src/pages/DirectoryPage/DirectoryPeople.js Outdated
Comment on lines +46 to +57
getFriends = () => {
fetch(`http://127.0.0.1:8000/profile`, {
headers: {
Authorization: `Token ${localStorage.getItem("token")}`
}
})
.then(res => res.json())
.then(json => {
this.makeFriends(json)
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be in a service to

Comment on lines +113 to +124
otherCards = (arr) => {
let length = others.length
for (let i = 0; i < length; i++) {
//same as the friends function
arr.push(
<div className="CommunityPage__momCard-single">
<PersonCard image={this.state.others[i].image} name={this.state.others[i].first_name} location="Washington D.C." />
<button className="FriendButton" onClick={this.sendRequest}>Add Friend</button>
</div>
)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here re array method and putting this in the render method

}
}
let data = { connections: connectionsList }
fetch(`http://127.0.0.1:8000/profile/${this.props.userid}`, {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you put this in a service and use the base_url constant? Otherwise, this will break in production

.catch(error => {
console.error('Error:', error);
});
window.location.reload(true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nope. Nope. Nope.

Please use react router

Comment thread client/src/pages/DirectoryPage/DirectoryPeople.js Outdated
Comment on lines +180 to +208
sendRequest = (e) => {
let newFriend = e.target.parentElement.children[0].children[1].textContent
e.target.parentElement.children[1].textContent = "Request Sent"
let newFriendId
let requestList
for (let i = 0; i < this.state.others.length; i++) {
if (this.state.others[i].first_name === newFriend) {
newFriendId = this.state.others[i].user
requestList = this.state.others[i].requests
}
}
requestList.push(this.props.userid)
let data = { requests: requestList }
fetch(`http://127.0.0.1:8000/profile/${newFriendId}`, {
method: 'PATCH',
headers: {
Authorization: `Token ${localStorage.getItem("token")}`,
'Content-Type': 'application/json',
},
body: JSON.stringify(data)
})
.then(response => response.json())
.then(result => {
console.log('Success:', result);
})
.catch(error => {
console.error('Error:', error);
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should get the users's id that the currently signed in user wants to friend and send a request to the backend with just that. All you need is the user ID for the friend request and the currently signed in user

@ccorkery97
Copy link
Copy Markdown
Author

ccorkery97 commented Jul 14, 2020

I fixed some of the minor changes but wasn't sure what you wanted with the services, did you want me to make my own services for connections and add those functions in there?

I did try putting the different cards into the render method but i couldn't get it to work and display each user in the array.

With the request functionality I wasn't sure how to set up the backend so that the request array would have not only the id the request is from but also if it is accepted or not, so i just have and array of id's and when one is accepted it is removed from the array.

@ccorkery97 ccorkery97 requested a review from 0xZakk July 14, 2020 19:52
@ccorkery97
Copy link
Copy Markdown
Author

Before this PR can be merged: functions should be added to services, JSX should be put inside render, and the logic of the functionality should be done in such a way where requests have an ID field denoting from which user the request is and also a boolean field showing whether the request has been accepted or not.

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.

2 participants