Skip to content

Conversation

@bakobagassas
Copy link
Contributor

@bakobagassas bakobagassas commented Jul 1, 2025

We should not be able to save a Bonner Requirement with an empty frequency. If we want it to be optional, lets make that clear. Issue with trying to modify the requirement section, along with Dr. Heggen, not liking the general functionality of how we save the rows and add them.
Fixes #1468

Issue with selecting the requirement section in Bonner management, clearing up misunderstanding of allowing frequency to be empty, issues with the process of saving and creating rows
Changes

  • Removed save changes button from bonnerManagement.HTML
  • Added new save button to HTML file
  • Created a saveRequirement function in bonnerManagement.js to handle both creating and updating rows
  • Created an addRequirement function to handle the creation of a new row by setting placeholder IDs to new rows
  • Created a remove button event handler to delete a specific row in the JavaScript file
  • #reqAdd is now disabled once a new row is created in the JavaScript file
  • created /deleteReq route to handle deleting rows, and modified /saveRequirements/ route to handle both new and updating rows
  • Created a deleteRequirement function in certification.py to delete the row and return if successful or not
  • Modified updatecertRequirements to handle creating new rows with placeholder IDs or updating previous rows and replacing old information
  • Modified the testing file for certificate and removed tests we were not using.

Testing

(If while trying to see our changes, you notice that the buttons on the bonner management page are not working then it is probably an issue with the symlink. You can solve that on your side by removing sortable.min.js and then doing the command ln -s sortable-1.15.0.min.js sortable.min.js in the VSCode terminal.

  • Go into admin > Bonner Management > Requirements
  • Click on add requirements
  • Enter the information required
  • Click the save button on that specific new row
  • Toast message will appear on the top right if successful, along with the page will reload with the new row already there
  • To test updating a row, change any information in a previous row and click save to update that row
  • To test the removal of a row, click the remove button on any row, and the page will refresh with that row being gone.

youremai and others added 30 commits June 14, 2025 21:48
… int instead of string. We worked with bonnerManagement.js in creating a new ajax and a new function to save the row instead of all rows.
… is somehow not defined. commented out code under document.ready() in bonnerManagement.js like ajax. Created a new save requirement function in certification.py.
…using jquery, worked on making changes to our route by uncommenting old code in order for our new row to save, and are currently working in certification.py to fix our issue of not saving.
…to work with old rows, and fixed functionality of updateCertRequirements to handle old rows.
…uplication issue, cleaned up and short code created.
transaction.rollback()

@pytest.mark.integration
def test_updateCertRequirements():
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this entire test removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the function that was being tested has been changed and is not fruitful anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

But the function still exists. It still needs a test. Also, there is no test of your new delete function

transaction.rollback()

@pytest.mark.integration
def test_updateCertRequirements():
Copy link
Contributor

Choose a reason for hiding this comment

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

But the function still exists. It still needs a test. Also, there is no test of your new delete function

if deleteReq == True:
return jsonify({"success": True})
else:
return jsonify({"success": False, "message": "Failed to delete requirement."})
Copy link
Contributor

Choose a reason for hiding this comment

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

No success: True or success: False please. Use statuses for failure where possible

In addition, if they ask to delete something and it doesn't exist, who cares? They didn't want it in the first place. That should still count as succeeding.

data: JSON.stringify(rowid),
success: function(response) {
msgToast("Bonner", "Removing Bonner Requirement");
location.reload();
Copy link
Contributor

Choose a reason for hiding this comment

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

Reloading is fine, but it would probably be cleaner to just delete the row from the table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we remove the reload, the row gets automatically deleted, but it doesn't display until we manually reload. Should we still delete the reload or keep it?

@github-actions
Copy link

View Code Coverage

@ojmakinde
Copy link
Contributor

Implementation in this PR has grown too complex from the initially simple issue. To solve this, simply prevent saving with an empty frequency

@ojmakinde ojmakinde closed this Sep 12, 2025
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.

Enforce Required Frequency on Bonner Requirements

10 participants