mentorship-backend icon indicating copy to clipboard operation
mentorship-backend copied to clipboard

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

Open isabelcosta opened this issue 4 years ago • 33 comments

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.
  • [x] Approval by 1 mentor.

Estimation

4 hours

isabelcosta avatar Jul 19 '20 18:07 isabelcosta

@isabelcosta I would like to work on this issue.

PrashanthPuneriya avatar Jul 21 '20 09:07 PrashanthPuneriya

@PrashanthPuneriya Are you available to work on this?

vj-codes avatar Jul 24 '20 15:07 vj-codes

@vj-codes Yes!

PrashanthPuneriya avatar Jul 25 '20 04:07 PrashanthPuneriya

@PrashanthPuneriya feel free to start working on this :)

isabelcosta avatar Jul 25 '20 11:07 isabelcosta

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

isabelcosta avatar Jul 28 '20 18:07 isabelcosta

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

PrashanthPuneriya avatar Jul 29 '20 07:07 PrashanthPuneriya

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 avatar Aug 01 '20 19:08 isabelcosta

@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?

jhalak27 avatar Aug 02 '20 15:08 jhalak27

@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?

isabelcosta avatar Aug 04 '20 18:08 isabelcosta

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

vj-codes avatar Aug 07 '20 09:08 vj-codes

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

jhalak27 avatar Aug 07 '20 13:08 jhalak27

@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.

isabelcosta avatar Aug 11 '20 18:08 isabelcosta

@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?

jhalak27 avatar Aug 15 '20 16:08 jhalak27

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.

yugantarjain avatar Aug 16 '20 12:08 yugantarjain

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

jhalak27 avatar Aug 18 '20 19:08 jhalak27

@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 :)

isabelcosta avatar Aug 21 '20 19:08 isabelcosta

@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 avatar Aug 21 '20 19:08 yugantarjain

@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

yugantarjain avatar Aug 21 '20 19:08 yugantarjain

Hey @jhalak27 any updates on this issue?

techno-disaster avatar Aug 24 '20 20:08 techno-disaster

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!

jhalak27 avatar Aug 26 '20 20:08 jhalak27

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

shubhank-saxena avatar Sep 01 '20 09:09 shubhank-saxena

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

PrashanthPuneriya avatar Sep 01 '20 12:09 PrashanthPuneriya

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 avatar Sep 08 '20 17:09 ashokrayal

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.

ashokrayal avatar Sep 08 '20 17:09 ashokrayal

Hey @Ashokrayal you can work on it.

shubhank-saxena avatar Sep 08 '20 17:09 shubhank-saxena

Assigning @Ashokrayal :)

PrashanthPuneriya avatar Sep 09 '20 12:09 PrashanthPuneriya

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 avatar Sep 09 '20 16:09 ashokrayal

@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.

PrashanthPuneriya avatar Sep 10 '20 08:09 PrashanthPuneriya

Hi @PrashanthPuneriya ,

I have fixed the issue and raised the PR.

Thanks.

ashokrayal avatar Sep 12 '20 11:09 ashokrayal

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

RiddhiAthreya avatar Jun 27 '21 10:06 RiddhiAthreya