Skip to content

fix: Return error message when relationship is not accepted in Comple… #549

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

fix: Return error message when relationship is not accepted in Comple… #549

wants to merge 6 commits into from

Conversation

StergiosSozos
Copy link
Contributor

@StergiosSozos StergiosSozos commented Apr 21, 2020 •
edited by isabelcosta
Loading

Description

The mentorship relation state (accepted or not) is now checked when someone is trying to complete a task from a relationship that is not accepted. This check is also of higher importance than the existence of the task. So if the relationship is not accepted the user gets a message that the relationship is not accepted.

Fixes #537

Type of Change:

  • Code

How Has This Been Tested?

All the tests have the same result as before. No extra tests were added.

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

Code/Quality Assurance Only

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

atm1504
atm1504 previously approved these changes Apr 22, 2020
View reviewed changes
isabelcosta
isabelcosta requested changes May 1, 2020
View reviewed changes
Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

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

@StergiosSozos I requested some minor changes. this is almost done, thank you for this contribution 🎉

isabelcosta
isabelcosta reviewed May 2, 2020
View reviewed changes
isabelcosta
isabelcosta requested changes May 2, 2020
View reviewed changes
Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

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

I just recalled now, can you create a simple test for it, showing that that is the first message that appears if the user attempts to create a task in a non accepted relation.
This would make this PR shine 🤩

@StergiosSozos
Copy link
Contributor Author

Sure, so a unit test right? So that we can test that the message that we want to be diaplayed is the one i added here and it does appear.

@isabelcosta
Copy link
Member

Sure, so a unit test right? So that we can test that the message that we want to be diaplayed is the one i added here and it does appear.

yes :)

@StergiosSozos
Test:Add test for complete_task when user's relation is pending
49e63eb 
Create 5th user
Create relationship between 4th and 5th user that is not accepted
Add test case to check the message shown
@StergiosSozos StergiosSozos requested a review from isabelcosta May 16, 2020 12:54
@StergiosSozos
Copy link
Contributor Author

A unit test for my method is now added, in addition with an extra user on the databse.

@devkapilbansal
Copy link
Member

@StergiosSozos can you please update this PR

@devkapilbansal devkapilbansal added the Status: Changes Requested Changes are required to be done by the PR author. label Feb 21, 2021
@isabelcosta
Copy link
Member

@StergiosSozos thank you for working on this project 🙌🏾 From what I see, only the merge conflicts are left to approve this. However, I will close this PR due to lack of response. If you wish to work on it again and the issue is still available feel free to comment there :)

@mariejp
Copy link

mariejp commented Apr 23, 2022

Hi! What were the requested changes for this PR? I cannot seem to locate them anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atm1504 atm1504 atm1504 left review comments

@isabelcosta isabelcosta Awaiting requested review from isabelcosta isabelcosta is a code owner

Assignees
No one assigned
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Complete task" not including all the necessary information when request id is not in accepted mode
5 participants
@StergiosSozos @isabelcosta @devkapilbansal @mariejp @atm1504