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

StudentTasksRequestHandler::applyRecordDelta() should re-use existing StudentTaskSkill records #769

Open
themightychris opened this issue Dec 31, 2022 · 0 comments
Labels
A-backend Area: Backend CBL models and request handlers C-chore Category: Necessary work for core-maintainers that is not a bug or feature E-easy Effort: Straightforward; recommended for a new contributor L-php Language: PHP

Comments

@themightychris
Copy link
Contributor

The following code block within StudentTasksRequestHandler::applyRecordDelta() will create fresh StudentTaskSkill records each time any changes to the list are saved:

foreach ($skills as $skillId => &$StudentTaskSkill) {
    $StudentTaskSkill = StudentTaskSkill::create([
        'SkillID' => $skillId
    ]);
}

$StudentTask->TaskSkills = array_values($skills);

It should instead have logic to re-use existing StudentTaskSkill records from a map of existing ones by skillId. This will reduce unnecessary churn of database rows and IDs

@themightychris themightychris added A-backend Area: Backend CBL models and request handlers E-easy Effort: Straightforward; recommended for a new contributor L-php Language: PHP C-chore Category: Necessary work for core-maintainers that is not a bug or feature labels Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend Area: Backend CBL models and request handlers C-chore Category: Necessary work for core-maintainers that is not a bug or feature E-easy Effort: Straightforward; recommended for a new contributor L-php Language: PHP
Projects
None yet
Development

No branches or pull requests

1 participant