Skip to content

[Bugfix:InstructorUI] Fix decoding of eg_thread_ids#11977

Merged
bmcutler merged 2 commits into
mainfrom
fix-prod
Aug 8, 2025
Merged

[Bugfix:InstructorUI] Fix decoding of eg_thread_ids#11977
bmcutler merged 2 commits into
mainfrom
fix-prod

Conversation

@williamschen23

@williamschen23 williamschen23 commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

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:

  1. Decoding it twice would allow for us to get [] instead of string "{}".
  2. Make a database migration for this situation where we specifically patch this one instance

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

@codecov

codecov Bot commented Aug 5, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.75%. Comparing base (47164b7) to head (2e6d152).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            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           
Flag Coverage Δ
autograder 21.31% <ø> (ø)
js 2.07% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.76% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@automateprojectmangement automateprojectmangement Bot moved this from Seeking Reviewer to In Review in Submitty Development Aug 6, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_ids from '{}'::json to '[]'::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 lavalleeale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have verified this fixes a frog robot, and the migration code lgtm

@github-project-automation github-project-automation Bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Aug 8, 2025
williamjallen pushed a commit that referenced this pull request Aug 8, 2025
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-&gt;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 />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=doctrine/orm&package-manager=composer&previous-version=2.19.6&new-version=3.5.0)](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]>
@bmcutler
bmcutler merged commit d31fdc3 into main Aug 8, 2025
30 checks passed
@bmcutler
bmcutler deleted the fix-prod branch August 8, 2025 17:27
@github-project-automation github-project-automation Bot moved this from Awaiting Maintainer Review to Done in Submitty Development Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants