Release 1.150.5 - #3595
Merged
Merged
Release 1.150.5#3595
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
OpenAPI ChangesShow/hide ## Changes for v0.yaml:Unexpected changes? Ensure your branch is up-to-date with |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
610
to
615
| }, | ||
| ], | ||
| }, | ||
| { | ||
| "name": "unique_app_id", | ||
| "label": "Unique App ID", | ||
| "description": "The unique app ID for the lineitem", | ||
| "groupName": "lineiteminformation", | ||
| "type": "string", | ||
| "fieldType": "text", | ||
| "hasUniqueValue": True, | ||
| "hidden": True, | ||
| }, | ||
| { | ||
| "name": "enrollment_mode", | ||
| "label": "Enrollment Mode", |
Contributor
There was a problem hiding this comment.
Bug: The unique_app_id property for line items is removed from the HubSpot schema but is still used when creating and searching, causing sync failures or duplicates.
Severity: HIGH
Suggested Fix
Either restore the unique_app_id property to the CUSTOM_ECOMMERCE_PROPERTIES["line_items"] definition or remove all usage of unique_app_id from the line item serialization and search logic, including LineSerializer and the _ensure_target_line_item_for_line function.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: hubspot_sync/api.py#L610-L615
Potential issue: The removal of the `unique_app_id` property definition from
`CUSTOM_ECOMMERCE_PROPERTIES["line_items"]` prevents its creation in new HubSpot
accounts. However, the application code, specifically `LineSerializer` and
`_ensure_target_line_item_for_line`, continues to send and search for line items using
this property. This discrepancy will cause the search function
`_find_target_line_item_id_by_unique_app_id` to fail silently by returning `None`.
Consequently, the sync process will attempt to create a new line item on every run,
leading to the creation of duplicate line items and data integrity issues.
Did we get this right? 👍 / 👎 to inform future reviews.
Comment on lines
+451
to
+455
| @extend_schema_field(ProgramPageSerializer(allow_null=True)) | ||
| def get_page(self, instance): | ||
| if hasattr(instance, "page"): | ||
| if hasattr(instance, "page") and instance.page is not None: | ||
| return ProgramPageSerializer(instance.page).data | ||
| else: | ||
| return {"feature_image_src": get_thumbnail_url(None)} | ||
| return None |
Contributor
There was a problem hiding this comment.
Bug: The ProgramSerializer now returns null for page instead of a fallback object, which will cause a frontend crash when accessing properties of program.page.
Severity: CRITICAL
Suggested Fix
Update ProgramSerializer.get_page() to return a default dictionary with expected keys (e.g., feature_image_src) when a program has no page, restoring the previous behavior and preventing frontend crashes. Alternatively, add null-checking in all frontend components that consume program.page.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: courses/serializers/v2/programs.py#L451-L455
Potential issue: The `ProgramSerializer.get_page()` method now returns `null` for
programs without a CMS page, where it previously returned a fallback object. Frontend
React components, such as `ProgramInfoBox.js`, expect `program.page` to be an object and
access its properties (e.g., `program.page.length`) without null-checking. This change
will cause a `TypeError: Cannot read properties of null` when rendering a program
without a page, crashing the React component tree and preventing the page from loading.
Did we get this right? 👍 / 👎 to inform future reviews.
odlbot
added
waiting for checkboxes
all checkboxes checked
and removed
deploying to rc
waiting for checkboxes
labels
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
6 participants
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.
annagav
Muhammad Arslan
cp-at-mit
Dan Subak
renovate[bot]
James Kachel