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

Api prog and meeting part 2 #194

Merged
merged 11 commits into from
Jan 12, 2020
Merged

Api prog and meeting part 2 #194

merged 11 commits into from
Jan 12, 2020

Conversation

sibsmc
Copy link
Collaborator

@sibsmc sibsmc commented Nov 23, 2019

Linked programming language and meeting interval information to api calls to api_user and wish.
Call only queries linked tables once to avoid multiple queries.
Filter now exists for true/false attributes on api_user and wish.
No tests have been written, no filters for string attributes.

@sibsmc sibsmc requested a review from a team as a code owner November 23, 2019 19:37
Copy link
Member

@nynnejc nynnejc left a comment

Choose a reason for hiding this comment

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

Tell me more about this! Which issue is this related to for context?

@sibsmc
Copy link
Collaborator Author

sibsmc commented Nov 24, 2019

No problem, the programming language and meeting interval are found in the relational diagram #12 . Before this update when you viewed a user and their wishes the programming language id and the meeting interval id would appear instead of the value it related to.
Another change that was made is that you can now filter on api_users according to whether they are a mentor or mentee, this links to #16 and #19

@sibsmc
Copy link
Collaborator Author

sibsmc commented Nov 24, 2019

Added new api_user attribute 'username'. Set both 'email' and 'username' as unique attributes. Updated tests. These changes address issue #195 . I have not done anything with passwords, which is also mentioned in #195, as authentication has not been dealt with yet. I will mark issue #195 as done and create a new issue for passwords using text from issue #195 'Password is not visible in plain text (user doesn't see the characters they type)'

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Hi! I was nearby and read a bit of the changes, and commented inline.

It's awesome to see stuff going on with the project!

server/app/models/api_user.rb Outdated Show resolved Hide resolved
server/spec/requests/api/v1/api_users_spec.rb Outdated Show resolved Hide resolved
Copy link
Member

@dennis dennis left a comment

Choose a reason for hiding this comment

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

Good work. There is a bunch of comments - some not related to you change directly, but while reading the code, I found them and would just let you know. You can ignore them, if its too much.

But I am impressed to see the amount of work and how many part of the framework that are in play. It's really promising :)

server/db/seeds.rb Show resolved Hide resolved
server/app/models/wish.rb Show resolved Hide resolved
@sibsmc sibsmc requested a review from dennis December 28, 2019 17:10
@sibsmc sibsmc added the backend Issue for backend team label Jan 1, 2020
Copy link
Member

@dennis dennis left a comment

Choose a reason for hiding this comment

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

Just a small gotcha, nothing that should stop you from merging

end

# context 'when email already in use' do
# let(:api_users.first
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to leave these here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These have been removed

@sibsmc sibsmc merged commit a0034fe into master Jan 12, 2020
@sibsmc sibsmc deleted the api_prog_and_meeting_part_2 branch January 12, 2020 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issue for backend team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants