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

The feature for batch adding transcript scores has been added. #1905

Open
wants to merge 3 commits into
base: v29.0.00
Choose a base branch
from

Conversation

AndroidOL
Copy link

Description
Adding a single score for exams like J.TEST, where the maximum score is 1000, is very cumbersome.

Motivation and Context
A new interface has been added to support entering the minimum and maximum values, with automatic reversal. Additionally, for alphabetic input, the system automatically converts the input to uppercase and supports adding a prefix.

How Has This Been Tested?
a-Z / z-A / 1 - 99999

Screenshots
iShot_2025-02-26_22 22 16

Comment on lines +35 to +38
$data = array('gibbonScaleID' => $gibbonScaleID);
$sql = 'SELECT name FROM gibbonScale WHERE gibbonScaleID=:gibbonScaleID';
$result = $connection2->prepare($sql);
$result->execute($data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hii @AndroidOL Thank you for implementing this feature request. While reviewing the code, I have found out that most of your sql statements are in the code itself and not in a respective Gateway class. Although, there are still a few modules that have similar style of code, we are trying to refactor these sql statements into respective Gateway classes for flexibility and easier maintenance. Thus, I would like to suggest you to put these sql statements into a Gateway class. Please feel free to refer to activities_manage.php and ActivityGateway.php file for reference on how to implement a Gateway class. Looking forward to the changes!

Copy link
Author

@AndroidOL AndroidOL Feb 27, 2025

Choose a reason for hiding this comment

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

A lot of code copy by gradeScales_manage_edit_grade_add.php🤣

Copy link
Contributor

@ali-ichk ali-ichk left a comment

Choose a reason for hiding this comment

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

Hii @AndroidOL Thank you for implementing this feature request. While reviewing the code, I have found out that most of your sql statements are in the code itself and not in a respective Gateway class. We realise we don't have lots of documentation on Gateway classes and although, there are still a few modules that have similar style of code, we are trying to refactor these sql statements into respective Gateway classes for flexibility and easier maintenance. Thus, I would like to suggest you to put these sql statements into a Gateway class. Please feel free to refer to activities_manage.php and ActivityGateway.php file for reference on how to implement a Gateway class. Looking forward to the changes!

@ali-ichk
Copy link
Contributor

ali-ichk commented Feb 27, 2025

Hi @AndroidOL Thank you for taking out the time to develop this new feature. It seems this will be quite useful to other schools especially the alphabetic input. However, to refer to your test case, where the maximum score is 1000, you may not be aware but you can actually use a built-in feature in Gibbon called "Enable Raw Attainment Marks" which is disabled by default to add a maximum score for each student. Again, we know that this is not well documented and probably this information is not out there. Go to Home > School Admin > Markbook Settings and enable the raw attainment marks. After that, you can add an "Attainment Total Mark" for a column in a Markbook. This is relevant to the test case you have provided. Nevertheless, the new feature request still seems useful and hopefully after a few tweaks can be good enough to merge into the core version. Appreciate the effort.

@AndroidOL
Copy link
Author

AndroidOL commented Feb 27, 2025

Hii @AndroidOL Thank you for implementing this feature request. While reviewing the code, I have found out that most of your sql statements are in the code itself and not in a respective Gateway class. We realise we don't have lots of documentation on Gateway classes and although, there are still a few modules that have similar style of code, we are trying to refactor these sql statements into respective Gateway classes for flexibility and easier maintenance. Thus, I would like to suggest you to put these sql statements into a Gateway class. Please feel free to refer to activities_manage.php and ActivityGateway.php file for reference on how to implement a Gateway class. Looking forward to the changes!

I don’t have much knowledge of programming, and I haven’t fully understood the code for Gibbon yet. I will try to rewrite the code using Gateway and attempt the PR again. This is a bit of a challenge for me, but I want to learn more about this area. Thank you for such a detailed reply.

@AndroidOL AndroidOL closed this Feb 27, 2025
@AndroidOL
Copy link
Author

Hi @AndroidOL Thank you for taking out the time to develop this new feature. It seems this will be quite useful to other schools especially the alphabetic input. However, to refer to your test case, where the maximum score is 1000, you may not be aware but you can actually use a built-in feature in Gibbon called "Enable Raw Attainment Marks" which is disabled by default to add a maximum score for each student. Again, we know that this is not well documented and probably this information is not out there. Go to Home > School Admin > Markbook Settings and enable the raw attainment marks. After that, you can add an "Attainment Total Mark" for a column in a Markbook. This is relevant to the test case you have provided. Nevertheless, the new feature request still seems useful and hopefully after a few tweaks can be good enough to merge into the core version. Appreciate the effort.

Yes, I'll try it.

@SKuipers
Copy link
Member

Hi @AndroidOL Thank you for your contributions! 😃 Gibbon is a massive codebase and we absolutely appreciate that our technical documentation is sparse (with such a small team), so we greatly appreciate your efforts to learn how to make changes.

Also, did you know you can push your new changes to the same branch to update your PR, rather than needing to close it and create a new one. It can help keep everything in one place.

@AndroidOL AndroidOL reopened this Feb 27, 2025
@AndroidOL
Copy link
Author

Hi @AndroidOL Thank you for your contributions! 😃 Gibbon is a massive codebase and we absolutely appreciate that our technical documentation is sparse (with such a small team), so we greatly appreciate your efforts to learn how to make changes.

Also, did you know you can push your new changes to the same branch to update your PR, rather than needing to close it and create a new one. It can help keep everything in one place.

Indeed, I wasn’t sure before, but this way, it makes it clearer to manage each functional requirement. I understand it now. Thanks again!

@SKuipers
Copy link
Member

SKuipers commented Mar 6, 2025

Hi @AndroidOL It looks like you may be committing multiple changes to the same branch. For PR's, a good practice is to use a separate branch for each set of changes, that way they can be reviewed, merged and tested separately.

As a note, there are importers in System Admin > Import from File to help with the bulk-creation of records, as often when a school is first setting up they have a large number of things to create, but then afterwards can simply use the UI. Be sure to check out these importers.

@AndroidOL
Copy link
Author

Hi @AndroidOL It looks like you may be committing multiple changes to the same branch. For PR's, a good practice is to use a separate branch for each set of changes, that way they can be reviewed, merged and tested separately.

As a note, there are importers in System Admin > Import from File to help with the bulk-creation of records, as often when a school is first setting up they have a large number of things to create, but then afterwards can simply use the UI. Be sure to check out these importers.

Thank you for your response.

This is my first time using GitHub, and at first, I didn't fully understand that I was using it incorrectly. Later, I realized that I couldn't submit a PR for new changes. After searching, I finally understood the concept of branches. However, I'm still unsure how to delete incorrect commits. I'll try to experiment with some operations later.

Regarding the second part of your response, while I completely understand your perspective, I prefer not to use table imports every time. For users like me, who don’t have much technical background, this approach isn’t very user-friendly.

Moreover, using a graphical interface can significantly reduce the chances of input errors. Even if an assessment is created incorrectly, it only affects the current score grade, which can be easily resolved by simply deleting the incorrect entry.

@AndroidOL
Copy link
Author

Hi @AndroidOL It looks like you may be committing multiple changes to the same branch. For PR's, a good practice is to use a separate branch for each set of changes, that way they can be reviewed, merged and tested separately.

As a note, there are importers in System Admin > Import from File to help with the bulk-creation of records, as often when a school is first setting up they have a large number of things to create, but then afterwards can simply use the UI. Be sure to check out these importers.

The excessive warnings in the file importer made me give up on using it. I hope there can be a faster start option for new Gibbon users.

A high entry barrier discourages users from adopting the system. Of course, you may believe that this process wouldn't be difficult for most users, but a smoother onboarding experience would still be beneficial.

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.

3 participants