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

feat: added target grade change history feature #44

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

nodogx
Copy link

@nodogx nodogx commented Nov 14, 2024

Description

This PR introduces a new Target Grade Change History feature on the front end and the back end. The feature includes:
• A new table in the Progress Dashboard to display the history of changes made to a student’s target grade.
• Integration with the back-end API to fetch and render the target grade history dynamically.
• Relevant CoffeeScript and HTML updates to ensure seamless functionality and responsiveness.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

The changes were tested as follows:

  1. Visual Validation:
    • Verified that the Target Grade Change History table renders correctly on the Progress Dashboard.
    • Ensured responsive layout across devices.
  2. API Integration Testing:
    • Confirmed data is fetched correctly from the back end and rendered in the front end.
    • Tested error handling for scenarios like no data or failed API calls.

Testing Checklist:

  • Tested in latest Chrome
  • Tested in latest Safari

Checklist:

My code follows the style guidelines of this project.
•	I have performed a self-review of my code.
•	I have commented my code, particularly in hard-to-understand areas.

Front end Pull Request -- thoth-tech/doubtfire-web#262

Screenshot 2024-11-14 at 7 35 26 pm

@nodogx nodogx changed the title Added target grade change history feature Feat: Added target grade change history feature Nov 14, 2024
@nodogx nodogx changed the title Feat: Added target grade change history feature feat: added target grade change history feature Nov 14, 2024
@nodogx nodogx closed this Nov 14, 2024
@nodogx nodogx reopened this Nov 19, 2024
Copy link

@Shounaks Shounaks left a comment

Choose a reason for hiding this comment

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

LGTM!

@aNebula
Copy link

aNebula commented Nov 25, 2024

LGTM!

Hi @Shounaks - please follow the PR review guide here https://thoth-tech.netlify.app/products/splashkit/splashkit-website/onboarding/05-peer-review/ (Point 1-3 are splashkit specific, but the rest of the guide should apply to OnTrack as well.)

In your review, you have to be more detailed than that - comment on what you looked at, what you checked, whether you cloned this and ran this (you must) and code quality and formatting etc.

Please review this again.

@washyking
Copy link

Don't know if this falls in the backend scope but can you maybe limit the number of grade changes viewed. Maybe max 10 or 20. so that it doesnt like change the styling of the whole ui. Than like a paginator. Apart from this its very good
Screenshot 2024-11-27 at 9 39 07 am

@nodogx
Copy link
Author

nodogx commented Nov 30, 2024

Don't know if this falls in the backend scope but can you maybe limit the number of grade changes viewed. Maybe max 10 or 20. so that it doesnt like change the styling of the whole ui. Than like a paginator. Apart from this its very good Screenshot 2024-11-27 at 9 39 07 am

This is something to do with fontend, which I have fixed with the new components. Now it shows 10 records per page.

@aNebula
Copy link

aNebula commented Dec 2, 2024

I am happy for a non-paginated backend api to be merged in this PR, as I've created a separate task for implementing backend pagination on the board.

@washyking
Copy link

I am happy for a non-paginated backend api to be merged in this PR, as I've created a separate task for implementing backend pagination on the board.

I am happy with your pull request if thats the case Jude good work.

Copy link

@washyking washyking left a comment

Choose a reason for hiding this comment

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

Good job

project.target_grade = new_grade

if project.save
# Record the grade change history

Choose a reason for hiding this comment

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

Like you commented the creation of target_grade_histories. Comments of what the attributes mean

@aNebula
Copy link

aNebula commented Dec 21, 2024

@nodogx If you have done API Integration testing, as mentioned in the description, why not include them with the code?

Please include tests, and then I'll review again.

@nodogx
Copy link
Author

nodogx commented Jan 5, 2025

@nodogx If you have done API Integration testing, as mentioned in the description, why not include them with the code?

Please include tests, and then I'll review again.

I have added the Test files @aNebula Could you please re-reveiw?

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.

4 participants