Conversation
BrianRamsay
changed the title
bakobagassas
requested review from
BrianRamsay
and removed request for
BrianRamsay
December 5, 2025 21:09
…the event table and the requiredTraining table
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Required training selections are not currently persisted/rehydrated server-side and the new RequiredTraining model has a verified Peewee backref-collision issue.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
app/controllers/admin/routes.py:348
trainingEventsincludes canceled and soft-deleted events; those shouldn’t be selectable as required trainings. Filter them out so the list only contains active training events.
trainingEvents = Event.select().where(Event.isTraining == True).order_by(Event.name)
- Files reviewed: 4/4 changed files
- Comments generated: 5
- Review effort level: Lite
Comment on lines
+4
to
+7
| class RequiredTraining(baseModel): | ||
|
|
||
| event = ForeignKeyField(Event) # The regular event that requires training | ||
| trainingEvent = ForeignKeyField(Event) # The event that is the required training. |
Comment on lines
+190
to
+191
| trainingEvents = Event.select().where(Event.isTraining == True).order_by(Event.name) | ||
|
|
Comment on lines
+894
to
+913
| // Handle "None Required" checkbox behavior | ||
| $('#noTrainingRequired').on('change', function() { | ||
| if ($(this).is(':checked')) { | ||
| // Uncheck all training checkboxes when "None Required" is selected | ||
| $('.training-checkbox').prop('checked', false); | ||
| } | ||
| }); | ||
|
|
||
| // Handle training checkbox behavior | ||
| $('.training-checkbox').on('change', function() { | ||
| if ($(this).is(':checked')) { | ||
| // Uncheck "None Required" when any training is selected | ||
| $('#noTrainingRequired').prop('checked', false); | ||
| } else { | ||
| // If no training checkboxes are checked, check "None Required" | ||
| if ($('.training-checkbox:checked').length === 0) { | ||
| $('#noTrainingRequired').prop('checked', true); | ||
| } | ||
| } | ||
| }); |
Comment on lines
+154
to
+155
| <label class="form-label" for="requiredTraining"><strong>Required Training</strong></label> | ||
| <div class="border rounded p-3" style="max-height: 200px; overflow-y: auto;"> |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical term-filtering and schema issues, along with persistence and form-handling defects, remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
app/controllers/admin/routes.py:191
- This query does not exclude soft-deleted or canceled training events. Those rows remain in
Eventand can be offered as requirements even though the training cannot be attended; filter to active events as the existing training query does.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- The edit query also includes soft-deleted and canceled training events, so obsolete trainings can be selected when editing an event. Apply the same active-event filters used by the training listing logic.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
eventDisplayserves both/viewand/edit, so this new query runs for every normal event view even thoughtrainingEventsis only passed to the edit template. In addition to the stale/non-unique 2024 lookup, a view request can now fail before rendering; build this list only for edit requests and use the event's term/current term.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/static/js/createEvents.js:899
- If the user starts with “None Required” checked and simply unchecks it without selecting a training, this handler leaves every checkbox unchecked. The form can therefore submit an empty selection even though the UI offers “None Required”; re-check this option when no training checkbox remains selected, or prevent it from being unchecked in that state.
$('#noTrainingRequired').on('change', function() {
if ($(this).is(':checked')) {
// Uncheck all training checkboxes when "None Required" is selected
$('.training-checkbox').prop('checked', false);
}
app/templates/events/createEvent.html:174
- These controls submit as
requiredTraining[], but the create/edit POST paths never read or persist that key:saveEventToDbonly copies the existing Event columns. As a result, selecting one or multiple trainings is discarded on save, andeventData.requiredTrainingcannot repopulate the edit form. Wire the selections to the relationship/model and read them withrequest.form.getlist('requiredTraining[]').
name="requiredTraining[]"
{{"checked" if eventData.requiredTraining and training.id|string in eventData.requiredTraining.split(',')}}>
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Lite
| if cohort: | ||
| bonnerCohorts[year] = cohort | ||
|
|
||
| isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024))) |
|
|
||
| rule = request.url_rule | ||
|
|
||
| isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024))) |
| isCanceled = BooleanField(default=False) | ||
| deletionDate = DateTimeField(null=True) | ||
| deletedBy = TextField(null=True) | ||
| requiresProgramTraining = BooleanField(default=False) |
…thub.com/BCStudentSoftwareDevTeam/celts into required_training_list_create_Event_1564
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Required-training options are not rendered or persisted correctly, and query, model, and schema issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
app/controllers/admin/routes.py:191
Term.get(Term.year == 2024)permanently limits the option source to the first 2024 term. The database already has Fall 2026 marked current, so create pages now show stale or empty training options, and a deployment without a 2024 term will fail. Use the applicable current/selected event term instead of a literal year.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- The edit path repeats the hard-coded
Term.year == 2024filter, so an existing event edited in the current or a future term receives the wrong training options as well. Share a term-aware training query with the create path rather than embedding a historical year here.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:191
- Events are soft-deleted through
deletionDate, but this query does not exclude them, so deleted training events will be offered as requirements. Add the sameEvent.deletionDate.is_null()predicate used bygetTrainingEvents.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- Events are soft-deleted through
deletionDate, but this query does not exclude them, so deleted training events will be offered as requirements in the edit form. Add the sameEvent.deletionDate.is_null()predicate used bygetTrainingEvents.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/models/event.py:32
requiresProgramTrainingis a single boolean, while the form accepts multiplerequiredTrainingvalues, and no code copies those values into this field or an association. The column therefore remains false and cannot represent the requested selections. Persist the selected training IDs through theRequiredTrainingrelation, or remove this placeholder until it is wired.
requiresProgramTraining = BooleanField(default=False)
app/models/event.py:32
- Adding this Peewee field without a migration leaves deployed/reset databases without the
requiresProgramTrainingcolumn. Because normalEventqueries select model fields, those environments will fail with an unknown-column error; include the corresponding migration/schema update before shipping the model change.
requiresProgramTraining = BooleanField(default=False)
app/models/requiredTraining.py:7
- Both foreign keys target
Eventwith the default back-reference, so importing this model gives Peewee two relationships with the same generated backref and can fail model initialization. Assign distinctbackrefvalues (or disable them) foreventandtrainingEvent.
event = ForeignKeyField(Event) # The regular event that requires training
trainingEvent = ForeignKeyField(Event) # The event that is the required training.
app/templates/events/createEvent.html:323
- These controls cannot represent or persist a selection: the first value is the event's boolean, the second is a stringified
Program, both checked states useisService, andpreprocessEventData/saveEventToDbignorerequiredTraining. A service event will therefore show both boxes and any submitted choices are discarded. Use stable training-event IDs, derive checked state from saved associations, and persist every selected ID.
<input class="form-check-input" type="checkbox" value="{{ eventData.isAllVolunteerTraining }}" id="allVolunteerTraining" name="requiredTraining" {{"checked" if eventData.isService}}>
<label class="form-check-label" for="noTrainingRequired"> All Volunteer Training </label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" value="{{ eventData['program'] }}" id="programSpecificTraining" name="requiredTraining" {{"checked" if eventData.isService}}>
app/templates/events/createEvent.html:324
- Both labels target
noTrainingRequired, which is not an input; clicking either visible option will not toggle its checkbox and assistive technology receives the wrong association. Point each label at its matching input,allVolunteerTrainingandprogramSpecificTraining.
<label class="form-check-label" for="noTrainingRequired"> All Volunteer Training </label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" value="{{ eventData['program'] }}" id="programSpecificTraining" name="requiredTraining" {{"checked" if eventData.isService}}>
<label class="form-check-label" for="noTrainingRequired"> {{eventData['program'].programName}} Training </label>
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Lite
Comment on lines
+319
to
+324
| <input class="form-check-input" type="checkbox" value="{{ eventData.isAllVolunteerTraining }}" id="allVolunteerTraining" name="requiredTraining" {{"checked" if eventData.isService}}> | ||
| <label class="form-check-label" for="noTrainingRequired"> All Volunteer Training </label> | ||
| </div> | ||
| <div class="form-check"> | ||
| <input class="form-check-input" type="checkbox" value="{{ eventData['program'] }}" id="programSpecificTraining" name="requiredTraining" {{"checked" if eventData.isService}}> | ||
| <label class="form-check-label" for="noTrainingRequired"> {{eventData['program'].programName}} Training </label> |
Comment on lines
+4
to
+7
| class RequiredTraining(baseModel): | ||
|
|
||
| event = ForeignKeyField(Event) # The regular event that requires training | ||
| trainingEvent = ForeignKeyField(Event) # The event that is the required training. |
Comment on lines
+895
to
+910
| $('#noTrainingRequired').on('change', function() { | ||
| if ($(this).is(':checked')) { | ||
| // Uncheck all training checkboxes when "None Required" is selected | ||
| $('.training-checkbox').prop('checked', false); | ||
| } | ||
| }); | ||
|
|
||
| // Handle training checkbox behavior | ||
| $('.training-checkbox').on('change', function() { | ||
| if ($(this).is(':checked')) { | ||
| // Uncheck "None Required" when any training is selected | ||
| $('#noTrainingRequired').prop('checked', false); | ||
| } else { | ||
| // If no training checkboxes are checked, check "None Required" | ||
| if ($('.training-checkbox:checked').length === 0) { | ||
| $('#noTrainingRequired').prop('checked', true); |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues affect training option rendering, persistence, schema compatibility, and form queries.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (13)
Previously missed (2) — in code that hasn't changed since the last review.
app/templates/events/createEvent.html:323
trainingEventsis computed and passed by both routes, but this block never iterates it; it hard-codes only two boolean values and does not render any training event IDs. As a result, the requested list of available trainings—including multiple choices and the de-duplicated options—is never shown. Render the supplied collection with stable option values and connect those values to the save path.
This issue also appears on line 319 of the same file.
app/templates/events/createEvent.html:324
- Both labels use
for="noTrainingRequired", but no control with that id exists and neither label matches its checkbox. Clicking the label will not toggle the new option and assistive technology will receive the wrong association; point each label at its corresponding input id.
app/controllers/admin/routes.py:191
- This hard-codes the training picker to the term whose
yearis 2024. It excludes current/future training events and will make event creation fail withDoesNotExistonce that term is unavailable; use the current academic year or the event's selected term instead.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- The edit route repeats the same literal 2024 filter, so editing an event in a later term also receives a stale/incomplete training list and can fail when that term is absent. Build this query from the current academic year or the event's selected term rather than a fixed year.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:191
order_by(Event.name)only sorts the rows; it does not remove duplicate training entries by name or any other training key. This query therefore does not implement the stated deduplication requirement before the list is rendered.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- The edit-path query also only sorts matching events and performs no deduplication, so it would reintroduce duplicate training options even if the create-path query were fixed.
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/models/event.py:32
- This new Peewee field is not present in the checked-in
eventschema (database/prod-backup.sql), andreset_database.sh from-backuprestores that dump without running the migration script. Event creation/update will therefore fail against a restored or otherwise un-migrated database when it writes this column; add the corresponding migration/schema update before using the field.
requiresProgramTraining = BooleanField(default=False)
app/models/requiredTraining.py:7
- Both foreign keys target
Eventwithout distinct backrefs. Peewee derives the default reverse accessor from the related model, so these relationships collide or become ambiguous when this model is imported. GiveeventandtrainingEventdistinct backrefs (or explicitly disable reverse accessors).
event = ForeignKeyField(Event) # The regular event that requires training
trainingEvent = ForeignKeyField(Event) # The event that is the required training.
app/models/requiredTraining.py:4
- This new model is not registered in
database/migrate_db.sh, so the documentedpem migrate/reset workflow will never create its table. No save path writes these relations either, so selections cannot be persisted; register and wire the model together, or defer adding it until that work is implemented.
class RequiredTraining(baseModel):
app/static/js/createEvents.js:900
- The new handlers target
#noTrainingRequiredand.training-checkbox, butcreateEvent.htmldefines neither selector for these inputs. The handlers therefore bind to empty sets, so selecting or clearing a training option never enforces the intended mutually exclusive behavior. Add matching markup/classes or update the selectors together with the template.
$('#noTrainingRequired').on('change', function() {
if ($(this).is(':checked')) {
// Uncheck all training checkboxes when "None Required" is selected
$('.training-checkbox').prop('checked', false);
}
});
app/templates/events/createEvent.html:319
- This checkbox submits
allVolunteerTraining, butsaveEventToDbreadsnewEventData['isAllVolunteerTraining'], so selecting it is discarded. The checked expression also readsallVolunteerTraining, which preprocessing creates as a separate default-false key, so existing all-volunteer events are not restored when edited. Use the persisted field name/key consistently.
<input class="form-check-input" type="checkbox" value="{{ eventData.isAllVolunteerTraining }}" id="allVolunteerTraining" name="allVolunteerTraining" {{"checked" if eventData.isService or eventData["allVolunteerTraining"] }}>
app/templates/events/createEvent.html:323
- The script binds to
#noTrainingRequiredand.training-checkbox, but neither selector exists in this markup; there is nonoTrainingRequiredcontrol and these inputs have notraining-checkboxclass. Consequently the none/one/multiple selection behavior never runs.
<input class="form-check-input" type="checkbox" value="{{ eventData.isAllVolunteerTraining }}" id="allVolunteerTraining" name="allVolunteerTraining" {{"checked" if eventData.isService or eventData["allVolunteerTraining"] }}>
<label class="form-check-label" for="noTrainingRequired"> All Volunteer Training </label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" value="{{ eventData['program'] }}" id="requiresProgramTraining" name="requiresProgramTraining" {{ "checked" if eventData.isService or eventData["requiresProgramTraining"] }}>
app/templates/events/createEvent.html:323
- Both checkboxes are forced checked whenever
eventData.isServiceis true, so every service event starts with both trainings selected. That prevents the requested optional "none" state and silently adds requirements the user did not choose.
<input class="form-check-input" type="checkbox" value="{{ eventData.isAllVolunteerTraining }}" id="allVolunteerTraining" name="allVolunteerTraining" {{"checked" if eventData.isService or eventData["allVolunteerTraining"] }}>
<label class="form-check-label" for="noTrainingRequired"> All Volunteer Training </label>
</div>
<div class="form-check">
<input class="form-check-input" type="checkbox" value="{{ eventData['program'] }}" id="requiresProgramTraining" name="requiresProgramTraining" {{ "checked" if eventData.isService or eventData["requiresProgramTraining"] }}>
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
| "rsvpLimit": newEventData['rsvpLimit'], | ||
| "contactEmail": newEventData['contactEmail'], | ||
| "contactName": newEventData['contactName'], | ||
| "requiresProgramTraining": newEventData['requiresProgramTraining'], |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues affect saving, schema migration, validation, and training controls.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (9)
Previously missed (2) — in code that hasn't changed since the last review.
app/templates/events/createEvent.html:317
- The route supplies
trainingEvents, but this block ignores that collection and renders only two hard-coded options. The resulting form therefore cannot display the queried training-event list or identify which training event was selected, so the requested list/multiple-training behavior is not implemented. Render the supplied collection (with an explicit deduplication rule) or remove the unused query and use a complete backing model.
app/templates/events/createEvent.html:320 - This label points to the nonexistent
noTrainingRequiredelement instead of the checkbox on line 319, so clicking the visible option does not toggle it and the label is not correctly associated for assistive technology. Point it atrequiresAllVolunteerTraining; the next label has the same issue.
app/controllers/admin/routes.py:191
Term.yearis shared by multiple terms, soTerm.get(Term.year == 2024)returns only one arbitrary 2024 term rather than the academic year; it also hard-codes a historical year. This makes the training collection incomplete (or empty once that term is absent) for current/future event creation. Scope the query to the relevant current or selected academic year/term instead.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/controllers/admin/routes.py:350
- This repeats the same broken
Term.get(Term.year == 2024)filter on the edit path: it selects one arbitrary term from 2024 and excludes current/future training events. Use the same relevant current/selected academic-year or term scope as the create path so editing does not show a different, stale list.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
trainingEvents = Event.select().where(isRelevantTraining).order_by(Event.name)
app/models/event.py:33
- These new Event columns are written by
saveEventToDb, but existing databases only receive model changes through the repository's Peewee migration workflow. Without applying a migration before deploying this code, the create and edit paths will issue SQL against missing columns and fail. Include or apply the schema migration as part of deployment.
requiresAllVolunteerTraining = BooleanField(default=False)
requiresProgramTraining = BooleanField(default=False)
app/models/event.py:55
- The key has an extra
r, socheckFlags()looks uprequiresProgramTrainingand never finds this rule. Rename the key to the actual field name so the intended validation is applied.
}
app/models/requiredTraining.py:7
- This new model is not registered in
database/migrate_db.sh, so the repository's reset/migration workflow will never create its table. No event create/update path usesRequiredTrainingeither, meaning this class currently cannot persist any selected training; wire and register the relationship or defer/remove this unused model until that work is implemented.
class RequiredTraining(baseModel):
event = ForeignKeyField(Event) # The regular event that requires training
trainingEvent = ForeignKeyField(Event) # The event that is the required training.
app/static/js/createEvents.js:898
- These handlers are bound to
#noTrainingRequiredand.training-checkbox, butcreateEvent.htmldeclares neither selector. jQuery silently binds to empty sets, so this entire none-versus-training behavior is inert; align the handlers with the actual controls and add the missing none option if that state is required.
$('#noTrainingRequired').on('change', function() {
if ($(this).is(':checked')) {
// Uncheck all training checkboxes when "None Required" is selected
$('.training-checkbox').prop('checked', false);
app/templates/events/createEvent.html:317
- The required-training section is inserted after the entire second-column description/contact/event-type area, while the start and end time controls are rendered earlier by
locationTimeMacroin the first column. It therefore is not under the start/end controls as required by issue #1564; move this block adjacent to the macro's time fields.
<!--Required Trainings-->
<div class="form-group mb-4">
<label class="form-label" for="requiredTrainings"><strong>Required Trainings</strong></label>
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+218
to
+219
| "requiresAllVolunteerTraining": newEventData['requiresAllVolunteerTraining'], | ||
| "requiresProgramTraining": newEventData['requiresProgramTraining'], |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved persistence, selector, relationship, migration, and term-selection issues block approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (8)
Previously missed (2) — in code that hasn't changed since the last review.
app/templates/events/createEvent.html:318
trainingEventsis passed from both controller paths, but this block never renders it; it only hard-codes two category checkboxes. Consequently the requested training-event list and duplicate handling are absent, and users cannot choose among the actual training options. Render a deduplicatedtrainingEventslist (or remove the unused query) and wire the selected event IDs into the persistence model.
app/templates/events/createEvent.html:324- Both labels in this group use
for="noTrainingRequired", but no element with that id is rendered. Clicking either label therefore does not toggle its checkbox, and assistive technology cannot associate the label with the control. Point each label at its corresponding input id.
app/controllers/admin/routes.py:190
- The production data contains Spring, Summer, and Fall 2024 terms, so a lookup keyed only by
year == 2024does not identify the intended term and permanently limits this list to stale 2024 data. Use the current academic year or the term selected for the new event instead of a hard-coded year.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
app/controllers/admin/routes.py:349
- The edit/view route repeats a lookup keyed only by
year == 2024; production has multiple 2024 terms, and the hard-coded year excludes current training events. Select the current academic year or the event's selected term here as well.
isRelevantTraining = ((Event.isAllVolunteerTraining | Event.isCeltsTraining | Event.isTraining) & ~Event.isLaborOnly & (Event.term == Term.get(Term.year == 2024)))
app/models/event.py:33
- These two Event columns are not accompanied by a schema migration. The deployed
eventtable does not contain them, butsaveEventToDbnow includes both keys in every insert/update, so production create/edit requests will fail with an unknown-column error. Add and apply the Peewee migration before using these fields.
requiresAllVolunteerTraining = BooleanField(default=False)
requiresProgramTraining = BooleanField(default=False)
app/models/requiredTraining.py:7
- Both foreign keys target
Eventwithout distinct backrefs, so Peewee gives them the same default reverse accessor. Consumers cannot distinguish the event that requires training from the event serving as the training. Configure separate backrefs for these relationships.
event = ForeignKeyField(Event) # The regular event that requires training
trainingEvent = ForeignKeyField(Event) # The event that is the required training.
app/models/requiredTraining.py:7
- This model is neither imported by the application nor registered in
database/migrate_db.sh, so the repository's migration workflow will not create its table and no selected training can be persisted through it. Either wire the model into the form/save path and migration, or remove this unused placeholder until the relationship is implemented.
class RequiredTraining(baseModel):
event = ForeignKeyField(Event) # The regular event that requires training
trainingEvent = ForeignKeyField(Event) # The event that is the required training.
app/static/js/createEvents.js:899
- Neither selector used here exists in the new template: there is no
#noTrainingRequiredinput and the two inputs have notraining-checkboxclass. These handlers therefore never run, so the advertised “None Required” mutual-exclusion behavior is nonfunctional. Add the missing control/class or update the selectors to match the rendered controls.
$('#noTrainingRequired').on('change', function() {
if ($(this).is(':checked')) {
// Uncheck all training checkboxes when "None Required" is selected
$('.training-checkbox').prop('checked', false);
}
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+218
to
+219
| "requiresAllVolunteerTraining": newEventData['requiresAllVolunteerTraining'], | ||
| "requiresProgramTraining": newEventData['requiresProgramTraining'], |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Description
Fixes #1564
-When we create an event, we should have a list of required training from which the user could choose one or more to complete the training(s). It should be under the start time and end time
Changes
TO DO NEXT:
-Create a dedicated RequiredTraining table: move training options out of the current hardcoded or ad hoc structure into their own persistent model.
-Refactor event-training relationship: establish a relationship between events and required trainings using a foreign key or many-to-many association.
-Link training selections in the event views: display the associated required trainings in event detail and summary views.