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

[7571] add ai feedback to comment #14

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

m4ra
Copy link
Contributor

@m4ra m4ra commented Aug 2, 2023

to be merged after this one #6
relevant files in this PR:

	modified:   adhocracy-plus/config/urls.py
	modified:   apps/moderatorfeedback/api.py
	modified:   apps/moderatorfeedback/serializers.py
	modified:   tests/moderatorfeedback/conftest.py
	modified:   tests/moderatorfeedback/test_moderator_comment_feedback_api.py
	added:      apps/ai_reports/serializers.py

respective api urls:

http://localhost:8004/api/comments/<comment_pk>/aicomments/
http://localhost:8004/api/contenttypes/<contant_type_pk>/objects/<comment_pk>/comments/<comment_pk>/

need to create an ai_report for testing purposes, see the test i added.
@hklarner need help with debugging why ai feedback still not part of the comment serializer, while I can access the ai_feedback api response alone. see screenshots:
image
image

@m4ra m4ra requested review from hklarner and goapunk August 2, 2023 11:33
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Coverage report

Total coverage

Status Category Percentage Covered / Total
🔴 Statements 12.62% 90/713
🔴 Branches 14.84% 54/364
🔴 Functions 10.42% 27/259
🔴 Lines 24.49% 569/2323

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 45d197a

@goapunk goapunk force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from bc2c081 to b0cad2d Compare August 9, 2023 09:17
@goapunk goapunk force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from b0cad2d to d0b62c0 Compare August 10, 2023 11:48
@hklarner
Copy link
Contributor

hklarner commented Aug 10, 2023

@hklarner need help with debugging why ai feedback still not part of the comment serializer, while I can access the ai_feedback api response alone.

You added ai_feedback to the CommentWithFeedbackSerializer but the Comment model does not have a field ai_feedback so this spec gets silently ignored. Basically the thing that does the mapping between serializer fields and the underlying model fields is the ModelSerializer (it is inherited: CommentWithFeedbackSerializer -> CommentSerializer -> ModelSerializer).

I would suggest to rename this field to ai_report:

class CommentWithFeedbackSerializer(a4_serializers.CommentSerializer):
    ai_report = AiReportSerializer(read_only=True)

because it introduces less vocabulary and renaming when we talk about things.

If you want to rename the serialized field you can also specify the source:

class CommentWithFeedbackSerializer(a4_serializers.CommentSerializer):
    ai_feedback = AiReportSerializer(read_only=True, source="ai_report")

@m4ra
Copy link
Contributor Author

m4ra commented Aug 14, 2023

@hklarner

You added ai_feedback to the CommentWithFeedbackSerializer but the Comment model does not have a field ai_feedback so this spec gets silently ignored. Basically the thing that does the mapping between serializer fields and the underlying model fields is the ModelSerializer (it is inherited: CommentWithFeedbackSerializer -> CommentSerializer -> ModelSerializer).

indeed, but the moderator_feedack field is also not present in the Comment model, so I don't get how that one works. Anyhow I resolved to use DRF relation serializers, in this case, the StringRelatedField since ai_report has a one-to-one relation with a comment, and the renaming of the serializer field to ai_reportworked because this is the name of the reverse relation we specified in the AiReport model. Thanks for pointing out the name change.

@m4ra m4ra force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from d0b62c0 to af6a70f Compare August 14, 2023 12:19
@m4ra m4ra changed the title WIP [7571] add ai feedback to comment [7571] add ai feedback to comment Aug 14, 2023
@m4ra m4ra force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from af6a70f to 562d44f Compare August 14, 2023 12:25
@m4ra
Copy link
Contributor Author

m4ra commented Aug 14, 2023

@goapunk can you take over the review on this one, since Hannes is away?

@goapunk
Copy link
Contributor

goapunk commented Aug 14, 2023

@goapunk can you take over the review on this one, since Hannes is away?

yes, will do (sorry, closed accidentally)

@goapunk goapunk closed this Aug 14, 2023
@goapunk goapunk reopened this Aug 14, 2023
apps/userdashboard/api.py Outdated Show resolved Hide resolved
@m4ra m4ra force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from 562d44f to eb613c5 Compare August 14, 2023 14:37
@m4ra m4ra force-pushed the mk-2023-07-add-ai-feedback-to-comment branch from eb613c5 to 45d197a Compare August 14, 2023 14:50
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

nice, looks good to me!

@goapunk goapunk enabled auto-merge (rebase) August 14, 2023 14:53
@goapunk goapunk merged commit d121bb6 into main Aug 14, 2023
2 of 3 checks passed
@goapunk goapunk deleted the mk-2023-07-add-ai-feedback-to-comment branch August 14, 2023 15:02
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.

None yet

3 participants