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

Improve GET (...)/comments response to include user.name and sent_by_me #685

Open
1 of 4 tasks
isabelcosta opened this issue Jul 19, 2020 · 33 comments
Open
1 of 4 tasks
Assignees
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Enhancement New feature or request.
Milestone

Comments

@isabelcosta
Copy link
Member

isabelcosta commented Jul 19, 2020

Description

As a frontend developer,
I need to have more information about the user who commented on a task,
so that I can display information as the user's name.

The idea is to improve the response of POST (...)/comments API to provide user name and sent_by_me boolean.

Mocks

Instead of having:

{
  "id": 0,
  "user_id": 0,
  "task_id": 0,
  "relation_id": 0,
  "creation_date": 0,
  "modification_date": 0,
  "comment": "string"
}

Have something like:

{
  "id": 0,
  "sent_by_me": true,
  "user": {
    "id": 0,
    "name": "string"
  },
  "task_id": 0,
  "relation_id": 0,
  "creation_date": 0,
  "modification_date": 0,
  "comment": "string"
}

where author is the person who commented.

Acceptance Criteria

Update [Required]

  • Update the response
  • Update and write new unit tests

Definition of Done

  • All of the required items are completed.
  • Approval by 1 mentor.

Estimation

4 hours

@isabelcosta isabelcosta added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Available Issue was approved and available to claim or abandoned for over 3 days. Type: Enhancement New feature or request. labels Jul 19, 2020
@PrashanthPuneriya
Copy link
Contributor

@isabelcosta I would like to work on this issue.

@vj-codes
Copy link
Member

@PrashanthPuneriya Are you available to work on this?

@PrashanthPuneriya
Copy link
Contributor

@vj-codes Yes!

@isabelcosta
Copy link
Member Author

@PrashanthPuneriya feel free to start working on this :)

@isabelcosta isabelcosta removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Jul 25, 2020
@isabelcosta
Copy link
Member Author

@PrashanthPuneriya are you still working on this :) Do you need any help?

@PrashanthPuneriya
Copy link
Contributor

@isabelcosta I was quite busy until now. Would resume working on this issue and ping you whenever I need help :)

@isabelcosta
Copy link
Member Author

No problem, but It's best then to open this issue for other contributors and then you can come back to this if its still available @PrashanthPuneriya

@isabelcosta isabelcosta added the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Aug 1, 2020
@isabelcosta isabelcosta pinned this issue Aug 2, 2020
@jhalak27
Copy link

jhalak27 commented Aug 2, 2020

@isabelcosta I would love to work on this. But I don't have much idea about this project. Could you give me some advice to get started?

@isabelcosta
Copy link
Member Author

isabelcosta commented Aug 4, 2020

@jhalak27 I would look at how other APIs are done and the API you are planning to change. The important part is the TaskCommentDAO, here you can modify the returns to include the data you want. For example, here https://github.com/anitab-org/mentorship-backend/blob/develop/app/api/dao/task_comment.py#L100 you see that the comments are being fetched, you need to get the name of the user using UserModel and add that to each comment that is in the list.

Does this help?

@vj-codes
Copy link
Member

vj-codes commented Aug 7, 2020

@jhalak27 would you like to work on this? Isabel has given you a brief about this issue above:)

@jhalak27
Copy link

jhalak27 commented Aug 7, 2020

@vj-codes @isabelcosta as I am new to this project, it will take some time. Will that be okay?

@isabelcosta isabelcosta removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Aug 11, 2020
@isabelcosta
Copy link
Member Author

@jhalak27 that is totally fine, just let us know when you are stuck. Please give us update within 3 days intervals, and we can help you.

@jhalak27
Copy link

@isabelcosta I am a bit confused about how to do this.
Do I need to make changes in the class TaskCommentModel?
https://github.com/jhalak27/mentorship-backend/blob/b9ebc706ceca1aad88fe4561d896e3a06c2f8a78/app/database/models/task_comment.py#L41 Here do I need to change the json object as desired?

Could you explain a bit more?

@yugantarjain
Copy link

yugantarjain commented Aug 16, 2020

It can also be considered to just add the user_name property instead of creating a new user object. That way, it won't be a breaking change for the frontend apps. However, that would be a small thing to fix on the frontend, and creating a user object as described in the description would be the more scalable option.

@jhalak27
Copy link

@yugantarjain I am still finding it a bit tough. What should I do?

@isabelcosta
Copy link
Member Author

@jhalak27 I am a bit busy, I will help you more in the next few days!
Also, feel free to ask for help on Zulip so that other people can try to help you out :)

@yugantarjain
Copy link

yugantarjain commented Aug 21, 2020

@yugantarjain I am still finding it a bit tough. What should I do?

Hi @jhalak27!

What exact problem are you facing? (I'm not a backend expert though)
Also, sorry for the late reply, and thanks for working on the issue!
Yeah, asking on Zulip would be nice.

@yugantarjain
Copy link

yugantarjain commented Aug 21, 2020

@isabelcosta I am a bit confused about how to do this.
Do I need to make changes in the class TaskCommentModel?
https://github.com/jhalak27/mentorship-backend/blob/b9ebc706ceca1aad88fe4561d896e3a06c2f8a78/app/database/models/task_comment.py#L41 Here do I need to change the json object as desired?

Could you explain a bit more?

Yes, adding a property (or the user object) around lines 23-29. And then the initializer will also have to be updated accordingly.
I'm not a backend expert, but most probably a migration script will also have to be created. Good Resource

@techno-disaster
Copy link

Hey @jhalak27 any updates on this issue?

@jhalak27
Copy link

It's bit hard for me. But I am trying to understand what all changes need to be done.
I think it would be better if you could open this issue for other contributors!

@shubhank-saxena
Copy link

@isabelcosta is this up for grabs? I would definitely like to give it a try !

@PrashanthPuneriya
Copy link
Contributor

Sure @shubhank-saxena :) Assigning you this issue. Happy coding!

@ashokrayal
Copy link
Contributor

Hi @PrashanthPuneriya ,

There is no activity on this issue. If this is available, I would like to work on this issue. I have one assigned issue which is resolved and I have created PR which is in review state.

@ashokrayal
Copy link
Contributor

Hi @PrashanthPuneriya ,

There is no activity on this issue. If this is available, I would like to work on this issue. I have one assigned issue which is resolved and I have created PR which is in review state.

@shubhank-saxena, Please let me know if you are still working on it.

@shubhank-saxena shubhank-saxena removed their assignment Sep 8, 2020
@shubhank-saxena
Copy link

Hey @ashokrayal you can work on it.

@PrashanthPuneriya
Copy link
Contributor

Assigning @ashokrayal :)

@ashokrayal
Copy link
Contributor

ashokrayal commented Sep 9, 2020

Hi @PrashanthPuneriya, @isabelcosta,

Can you please review this piece of code? I have not written testcases yet as I am not sure if this is right way to do it.
link

Thanks

ashokrayal referenced this issue in ashokrayal/mentorship-backend Sep 9, 2020
@PrashanthPuneriya
Copy link
Contributor

@ashokrayal I don't have much idea about Flask-RestX but, I added a review(note:- I may be wrong). Also, I think it is better if you create a PR even though if the work isn't completed and ask for further clarifications.

ashokrayal added a commit to ashokrayal/mentorship-backend that referenced this issue Sep 12, 2020
@ashokrayal
Copy link
Contributor

ashokrayal commented Sep 12, 2020

Hi @PrashanthPuneriya ,

I have fixed the issue and raised the PR.

Thanks.

@vj-codes vj-codes added the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Mar 17, 2021
@gaurivn gaurivn added this to the 1st MVP milestone Jun 12, 2021
@RiddhiAthreya
Copy link
Contributor

@isabelcosta looks like issue #1110 and this issue are trying to accomplish the same thing. Please have a look at it.

@diananova
Copy link
Contributor

This issue is not available, right?

@devkapilbansal devkapilbansal removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Jul 25, 2021
@devkapilbansal
Copy link
Member

@RiddhiAthreya assigned you this one as both issues were same

@mariejp
Copy link

mariejp commented Apr 23, 2022

I was not able to find any test cases that relied on the subfields in the TaskCommentModel. Am I correct in assuming that no test cases need to be updated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Type: Enhancement New feature or request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.