fix(task): resolve feedback notification issue

[amriith]

Description

Fixed an issue where feedback notifications were not sent when task status updated. Now, emails are correctly triggered based on status changes.

Fixes # (issue)

Fixes the Notification Mailer feature which notify students if the task status is changed into either of these (redo, fail, fix_and_resubmit, feedback_exceeded, discuss, demonstrate, complete)

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Test A
• Verified the Send Notifications is set as True for a particular unit by running an SQL Query
• Verified the Student has turned on their notification from the /edit-profile route
• Logged in as a tutor , selected the unit , selected the student assignment and marked it as fail
• Once the task status is marked as failed the notification will be sent to the student.
• I have currently set it to save it as a file in your doubtfire-api/tmp/mails and have updated it in config/environments/development.rb file
• The email can be viewed here in /tmp/mails with all the necessary details in it

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if appropriate
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have created or extended unit tests to address my new additions
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

If you have any questions, please contact @macite or @jakerenzella.

[martindolores]

Looking good so far! Able to replicate testing steps for the required outcome but some things to note here 🤘

[martindolores]

Not 100% sure if this file (Gemfile.lock) should be committed - please double check this

[amriith]

The changes in Gemfile.lock weren't intentionally added by me - they were automatically generated when I ran the app locally. These changes add x86_64-Linux platform support alongside the existing aarch64 platform.

From what I understand, this is generally beneficial as it expands compatibility to both architectures, but I wanted to confirm if this aligns with our project standards before committing these changes. If preferred, I can revert these Gemfile.lock changes.

[martindolores]

What you say seems good to me! Lets double check with Nebula tomorrow during standup but that sounds fine :)

[b0ink]

These packages are present in upstream

I recommend merging your branch with 8.0.x and changing the base branch on this PR to 8.0.x

[amriith]

Hi @b0ink I have created a new pull request as suggested from 8.0.x branch. https://github.com/thoth-tech/doubtfire-api/pull/55/files

[martindolores]

Hi @amriith 😄, you can changes the base branch on this PR as so without having to open up a new PR:

It also looks like some files that you have removed has been added in and some merge conflicts on the other PR:

I suggest to just close the other PR and change the base branch on this to 8.0x. Note that you will have to merge 8.0x to this branch (just run git merge 8.0x and fix any conflicts)

[martindolores]

This is a config file - changes to this are typically not something you would commit unless they are part of a necessary configuration

If testing your changes, just leave in local repo without committing i.e. dont include in this PR

[amriith]

I've removed the configuration file from this PR as suggested, Thanks for letting me know.

[amriith]

Made all the mentioned changes in development.rb and task.rb file

[amriith]

Base branch changed and merged current branch to 8.0.x

[martindolores]

Great work! 🕺

[AB-Deakin]

Tested in chrome and firefox (no safari access). Attached screenshots of success both with chrome and firefox updated feedback and generated test email. No issues with code.

[AB-Deakin]

Tested and working correctly. Peer Review approved by Alex Brown

[aNebula]

Please resolve conflicts and take the PR through the review process (2 peers, followed by 1 senior leader) again.

[amriith]

Hi @AB-Deakin @martindolores ,
I had a few minor merge conflicts (mainly in the Gemfile) which I’ve resolved.
Could you please run a quick test and review it when you get a chance? Thanks!

[AB-Deakin]

Hi @amriith I've gone over the changes and tested and no issues with running it and code looks all good from my checks peer review approved

[AB-Deakin]

Hi @amriith I've gone over the changes and tested and no issues with running it and code looks all good from my checks peer review approved

[martindolores]

Hi @amriith, changes look good to me. No issues running this 🙂

[aNebula]

LGTM.
@amriith Please open a PR against doubtfire-lms/doubtfire-api with 9.x branch.
Remember to include description and context.

[amriith]

Hi @aNebula Opened a new pr on doubtfire-lms/doubtfire-api with 9.x branch

url- doubtfire-lms#471