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

Selectively remove service transcript entry for banned programs #1176

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

Conversation

gahimbaref
Copy link
Contributor

@gahimbaref gahimbaref commented Apr 9, 2024

Issue: #658
Add a checkbox to the ban modal dialog to choose whether to completely remove the program from the service transcript or keep it in the service transcript (in case if the checkbox is not clicked). The checkbox on the modal should affect the service transcript directly and remove or keep the programs, whose modals were checked/unchecked.

Did:

  • Added a checkbox for banned modals (It is shown only on the Unban version and hidden on the Ban version)
  • Added a new boolean column removeFromTranscript in programBan table
  • Added a new route to update programBan table when checkbox state changes
  • The fixes are reflected on the database and the service transcript page

Test:

  • checkout the branch serviceTEntry658
  • reset the database and run flask
  • Go to student search and search for a student, like "khatts"
  • In the Choose Action dropdown, select view service transcript, note which programs appear on the transcript
  • Go back to the student profile, and under "Programs" in accordion, click on the "Edit" button that will open a modal Ban/Unban
  • Ban a student from another program like "Hunger Initiatives" (keep in mind that the program will show up in the service transcript only if the student earned hours for that program)
  • Select the "Remove From Transcript" checkbox on the modal that is on the stage of Unban, and then close the modal
  • Go back to view the service transcript, and check if that program appears (it should not appear)
  • Try to remove the checkbox and see if the program appears again on the service transcript
  • Review Code in these files: userProfile.js, userProfile.html, main/routes.py, transcript.py

@Karina-Agliullova Karina-Agliullova marked this pull request as ready for review April 9, 2024 21:15
@hoerstl
Copy link
Contributor

hoerstl commented Apr 11, 2024

Copy link
Contributor

@hoerstl hoerstl left a comment

Choose a reason for hiding this comment

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

Pretty good! I found a couple of bugs but they don't seem too difficult to iron out. Once you fix the non-displayed program hours ending up in the transcript total and that one modal-title selector changing all modal titles on the user's profile this looks good!

app/controllers/main/routes.py Outdated Show resolved Hide resolved
app/controllers/main/routes.py Outdated Show resolved Hide resolved
app/controllers/main/routes.py Outdated Show resolved Hide resolved
app/logic/transcript.py Outdated Show resolved Hide resolved
app/logic/transcript.py Outdated Show resolved Hide resolved
app/static/js/userProfile.js Outdated Show resolved Hide resolved
app/static/js/userProfile.js Outdated Show resolved Hide resolved
app/static/js/userProfile.js Outdated Show resolved Hide resolved
app/static/js/userProfile.js Outdated Show resolved Hide resolved
app/static/js/userProfile.js Outdated Show resolved Hide resolved
@bledsoef
Copy link
Contributor

So I found an issue of when you ban a user, remove that program from their transcript, and then unban the user, it is still removed from the transcript.

Copy link

View Code Coverage

Copy link
Contributor

@ojmakinde ojmakinde left a comment

Choose a reason for hiding this comment

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

The PR functionality and code look good. However, I have a few suggestions:

  • It would be good to add some confirmation after (un)removing a program from a student's transcript. The only way I could tell changes were being made was looking at the POST requests on my flask session. A flash or basic confirmation would improve feedback.
  • Using a radio instead of a checkbox would also look better.
  • Adding the "Remove from Transcript" to the main page instead of the Edit Modal would also make the functionality better. Greying out the function until the event is banned would be a viable way to implement its current functionality.

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.

6 participants