Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11977 +/- ##
=========================================
Coverage 21.75% 21.75%
Complexity 9459 9459
=========================================
Files 267 267
Lines 36146 36146
Branches 474 474
=========================================
Hits 7863 7863
Misses 27813 27813
Partials 470 470
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a data type inconsistency issue with the eg_thread_ids column in the electronic_gradeable table. The bug occurred because existing gradeables were created with a default value of "{}" (a JSON string) instead of [] (a JSON array), causing decoding issues in the application.
- Updates the default value for
eg_thread_idsfrom'{}'::jsonto'[]'::json - Adds a migration to fix existing records that have the incorrect string value
- Ensures consistent JSON array type for thread IDs across all gradeables
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| migration/migrator/migrations/course/20250804212849_default_thread_ids_value.py | Adds migration to update default value and fix existing records with incorrect JSON string format |
| migration/migrator/data/course_tables.sql | Updates table schema to use JSON array [] as default instead of JSON object {} |
lavalleeale
left a comment
There was a problem hiding this comment.
I have verified this fixes a frog robot, and the migration code lgtm
Bumps [doctrine/orm](https://github.com/doctrine/orm) from 2.19.6 to 3.5.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/doctrine/orm/releases">doctrine/orm's releases</a>.</em></p> <blockquote> <h2>3.5.0</h2> <h3>Release Notes for <a href="https://github.com/doctrine/orm/milestone/182">3.5.0</a></h3> <p>Feature release (minor)</p> <h3>3.5.0</h3> <ul> <li>Total issues resolved: <strong>0</strong></li> <li>Total pull requests resolved: <strong>9</strong></li> <li>Total contributors: <strong>5</strong></li> </ul> <h4>New Features</h4> <ul> <li><a href="https://redirect.github.com/doctrine/orm/pull/11988">11988: Add index mapping to column</a> thanks to <a href="https://github.com/jannes-io"><code>@jannes-io</code></a></li> <li><a href="https://redirect.github.com/doctrine/orm/pull/11781">11781: Detect DBAL's number type</a> thanks to <a href="https://github.com/derrabus"><code>@derrabus</code></a></li> </ul> <h4>Improvements</h4> <ul> <li><a href="https://redirect.github.com/doctrine/orm/pull/12037">12037: Add console completion for entityName param of orm:mapping:describe c…</a> thanks to <a href="https://github.com/stlgaits"><code>@stlgaits</code></a></li> <li>[11978: <a href="https://redirect.github.com/doctrine/orm/issues/11977">#11977</a> implemented batching of <code>INSERT</code> operations in <code>UnitOfWork#executeInserts()</code> so that <code>EntityPersister#executeInserts()</code> calls are reduced](<a href="https://redirect.github.com/doctrine/orm/pull/11978">doctrine/orm#11978</a>) thanks to <a href="https://github.com/Ocramius"><code>@Ocramius</code></a></li> </ul> <h4>Deprecations</h4> <ul> <li><a href="https://redirect.github.com/doctrine/orm/pull/12036">12036: Deprecate proxy autoloader and class name resolver</a> thanks to <a href="https://github.com/greg0ire"><code>@greg0ire</code></a></li> <li><a href="https://redirect.github.com/doctrine/orm/pull/12022">12022: Provide upgrade path to new ORMSetup::create* signature</a> thanks to <a href="https://github.com/greg0ire"><code>@greg0ire</code></a></li> <li><a href="https://redirect.github.com/doctrine/orm/pull/12020">12020: Deprecate more proxies-related methods or calls</a> thanks to <a href="https://github.com/greg0ire"><code>@greg0ire</code></a></li> <li><a href="https://redirect.github.com/doctrine/orm/pull/12005">12005: Deprecate not using native lazy objects on PHP 8.4+</a> thanks to <a href="https://github.com/greg0ire"><code>@greg0ire</code></a></li> <li><a href="https://redirect.github.com/doctrine/orm/pull/11989">11989: Deprecate methods for configuring no longer configurable features</a> thanks to <a href="https://github.com/greg0ire"><code>@greg0ire</code></a></li> </ul> <h2>3.4.4</h2> <h3>Release Notes for <a href="https://github.com/doctrine/orm/milestone/186">3.4.4</a></h3> <p>3.4.x bugfix release (patch)</p> <h3>3.4.4</h3> <ul> <li>Total issues resolved: <strong>0</strong></li> <li>Total pull requests resolved: <strong>4</strong></li> <li>Total contributors: <strong>3</strong></li> </ul> <h4>Bugfixes</h4> <ul> <li><a href="https://redirect.github.com/doctrine/orm/pull/12043">12043: Only throw PHP 8.4 requirement exception when enabling native lazy objects.</a> thanks to <a href="https://github.com/beberlei"><code>@beberlei</code></a></li> </ul> <h4>Improvement</h4> <ul> <li><a href="https://redirect.github.com/doctrine/orm/pull/12039">12039: do not register the legacy proxy class name resolver with enabled native lazy ghost</a> thanks to <a href="https://github.com/xabbuh"><code>@xabbuh</code></a></li> </ul> <h4>Static Analysis</h4> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Upgrade guide</summary> <p><em>Sourced from <a href="https://github.com/doctrine/orm/blob/3.5.x/UPGRADE.md">doctrine/orm's upgrade guide</a>.</em></p> <blockquote> <h1>Upgrade to 3.5</h1> <h2>Deprecate not using native lazy objects on PHP 8.4+</h2> <p>Having native lazy objects disabled on PHP 8.4+ is deprecated and will not be possible in 4.0.</p> <p>You can enable them through configuration:</p> <pre lang="php"><code>$config->enableNativeLazyObjects(true); </code></pre> <p>As a consequence, methods, parameters and commands related to userland lazy objects have been deprecated on PHP 8.4+:</p> <ul> <li><code>Doctrine\ORM\Tools\Console\Command\GenerateProxiesCommand</code></li> <li><code>Doctrine\ORM\Configuration::getAutoGenerateProxyClasses()</code></li> <li><code>Doctrine\ORM\Configuration::getProxyDir()</code></li> <li><code>Doctrine\ORM\Configuration::getProxyNamespace()</code></li> <li><code>Doctrine\ORM\Configuration::setAutoGenerateProxyClasses()</code></li> <li><code>Doctrine\ORM\Configuration::setProxyDir()</code></li> <li><code>Doctrine\ORM\Configuration::setProxyNamespace()</code></li> <li>Passing more than one argument to <code>Doctrine\ORM\Proxy\ProxyFactory::__construct()</code></li> </ul> <p>Additionally, some methods of ORMSetup have been deprecated in favor of a new counterpart.</p> <ul> <li><code>Doctrine\ORM\ORMSetup::createAttributeMetadataConfiguration()</code> is deprecated in favor of <code>Doctrine\ORM\ORMSetup::createAttributeMetadataConfig()</code></li> <li><code>Doctrine\ORM\ORMSetup::createXMLMetadataConfiguration()</code> is deprecated in favor of <code>Doctrine\ORM\ORMSetup::createXMLMetadataConfig()</code></li> <li><code>Doctrine\ORM\ORMSetup::createConfiguration()</code> is deprecated in favor of <code>Doctrine\ORM\ORMSetup::createConfig()</code></li> </ul> <h2>Deprecate methods for configuring no longer configurable features</h2> <p>Since 3.0, lazy ghosts are enabled unconditionally, and so is rejecting ID collisions in the identity map.</p> <p>As a consequence, the following methods are deprecated and will be removed in 4.0:</p> <ul> <li><code>Doctrine\ORM\Configuration::setLazyGhostObjectEnabled()</code></li> <li><code>Doctrine\ORM\Configuration::isLazyGhostObjectEnabled()</code></li> <li><code>Doctrine\ORM\Configuration::setRejectIdCollisionInIdentityMap()</code></li> <li><code>Doctrine\ORM\Configuration::isRejectIdCollisionInIdentityMapEnabled()</code></li> </ul> <h1>Upgrade to 3.4.1</h1> <h2>BC BREAK: You can no longer use the <code>.*</code> notation to get all fields of an entity in a DTO</h2> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/doctrine/orm/commit/6deec3655ba3e8f15280aac11e264225854d2369"><code>6deec36</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/12046">#12046</a> from greg0ire/3.5.x</li> <li><a href="https://github.com/doctrine/orm/commit/7f40422d21bddcad614740ffe99790f53d42b17e"><code>7f40422</code></a> Merge remote-tracking branch 'origin/3.4.x' into 3.5.x</li> <li><a href="https://github.com/doctrine/orm/commit/e67fa5388bd348b26fbbdb8d01eaf05fa9ac8d9c"><code>e67fa53</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/12043">#12043</a> from beberlei/Bugfix-DisableNativeLazyLogicException</li> <li><a href="https://github.com/doctrine/orm/commit/80053336c9c83f3375a0c5e1fdf99d4dbc39a65e"><code>8005333</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/12044">#12044</a> from doctrine/3.4.x</li> <li><a href="https://github.com/doctrine/orm/commit/dddcc507ef876e5fe34e9c04671c61776a02f00b"><code>dddcc50</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/12039">#12039</a> from xabbuh/pr-12036</li> <li><a href="https://github.com/doctrine/orm/commit/b41d9da88d7f319fd5cb14f440413714db1f502a"><code>b41d9da</code></a> do not register the legacy proxy class name resolver with enabled native lazy...</li> <li><a href="https://github.com/doctrine/orm/commit/c04bfb78b7aa672c74bd0d03ebbc5b9a19a25422"><code>c04bfb7</code></a> Only throw PHP 8.4 requirement exception when enabling native lazy objects.</li> <li><a href="https://github.com/doctrine/orm/commit/8a5dfc86d49fee86d08777df7f399b0f98b0019a"><code>8a5dfc8</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/12037">#12037</a> from stlgaits/mapping-describe-completion</li> <li><a href="https://github.com/doctrine/orm/commit/79e103c07e9d71a0b96d773013f2f919f2a17351"><code>79e103c</code></a> Merge pull request <a href="https://redirect.github.com/doctrine/orm/issues/11978">#11978</a> from Ocramius/feature/#11977-batch-handling-of-inse...</li> <li><a href="https://github.com/doctrine/orm/commit/5afadf163a3630830a61e4b7620991491fc1d7b8"><code>5afadf1</code></a> Add console completion for entityName param of orm:mapping:describe command</li> <li>Additional commits viewable in <a href="https://github.com/doctrine/orm/compare/2.19.6...3.5.0">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alex Lavallee <[email protected]>
Why is this Change Important & Necessary?
From #11702,
On production, we have gradeables that have already been created. When the gradeable gets created, we
json_encode('{}')as the default value. This means that when it gets inserted into the database, instead of{}being inserted, we actually insert"{}"instead, which is a string. This means that the new code, which decodes it once, will not work as it will be decoded to"{}"instead of[], which throws of frog robot.We can either:
[]instead of string"{}".What is the New Behavior?
Make a database migration for this situation where we specifically patch this one instance. I feel like this is better and the other way is a little hacky
What steps should a reviewer take to reproduce or test the bug or new feature?
Make a gradeable before #11702. Move to after #11702, edit gradeable, see error
Automated Testing & Documentation
This could not have been prevented that easily with e2e test. The initial state would have to be from the previous course creations, and then switching to the new version. We could do this to prevent this error, but it would be very hard.
Other information