apluslms / a-plus

A+ frontend portal - A+ LMS documentation:
https://apluslms.github.io/
Other
64 stars 72 forks source link

Update to Jutut that a feedback response has been read #1363

Closed Mikael-Lenander closed 1 month ago

Mikael-Lenander commented 2 months ago

Description

What?

Sends an update to Jutut when a student has read a feedback response.

Why?

For urgent messages, teachers need to know whether the student has read the message.

How?

Send a post request to Jutut endpoint /feedback/ that is responsible for creating and updating Feedback instances based on feedback submissions in A+. Adds an attribute to Submission serializer that indicates whether there are unread feedback responses for that (feedback) submission.

Fixes https://github.com/apluslms/mooc-jutut/issues/40.

Has to be merged with https://github.com/apluslms/mooc-jutut/pull/105.

Testing

Remember to add or update unit tests for new features and changes.

What type of test did you run?

Logged in with student and root. Sent a feedback with student and responded to it through Jutut with root. Opened the response notification in A+ and checked that the ok-icon appeared in Jutut. Same test on multiple feedbacks to the same assignment and on editing the response message.

Did you test the changes in

Think of what is affected by these changes and could become broken

Translation

Programming style

Have you updated the README or other relevant documentation?

Is it Done?

Clean up your git commit history before submitting the pull request!

etanttila commented 1 month ago

Small style nitpicks: Docstrings should begin right after the first triple quotes (no newline) A comma after the 'feedback_response_seen' would be nice

And speaking of docstrings, I also forgot to mention that "for long blocks of text (such as docstrings and comments), aim to keep the line length under 72 characters to make the text more readable for humans by limiting the column width" (from our styleguide), currently the line lengths are too long (over 100 chars).

jsorva commented 1 month ago

I also shortened the docstring lines to under 72 although I've always used 119 as the max line length myself. (It is easier to follow one length instead of multiple. I don't know why our style guide recommends using 72, 79, and 119)

I’m all for following team style guides.

Had I a vote, I’d vote—if those three are the options in 2024—for 119. Something approx. halfway between 79 and 119 would also feel right.

I don’t know where those three numbers came from. Presumably some Python conventions that I’m unaware of (which would be most of Python conventions).

(Not a team member. Just shouting from the sidelines.)