Skip to content

Conversation

@augusto-weiss
Copy link

No description provided.

Copilot AI review requested due to automatic review settings January 12, 2026 16:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request attempts to modify the OAuth provider scope data handling by keeping tuple values for many2one fields instead of extracting just the name component. However, this change introduces a critical bug that will cause runtime failures.

Changes:

  • Modified _get_data_for_model method to return full tuple values for many2one fields instead of extracting the name
  • Commented out the previous implementation that extracted the name from tuples
  • Added a comment explaining the intent to keep tuple values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +85 to +89
# if isinstance(value, tuple):
# # Return only the name for a many2one
# data[record_data["id"]][field] = value[1]
# else:
# data[record_data["id"]][field] = value
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

Commented-out code should be removed entirely rather than left in the codebase. If the old implementation needs to be referenced, it can be retrieved from version control history.

Suggested change
# if isinstance(value, tuple):
# # Return only the name for a many2one
# data[record_data["id"]][field] = value[1]
# else:
# data[record_data["id"]][field] = value

Copilot uses AI. Check for mistakes.
Comment on lines +83 to 90
# m2o filds return a tuple (id, name), now we want keep it
data[record_data["id"]][field] = value
# if isinstance(value, tuple):
# # Return only the name for a many2one
# data[record_data["id"]][field] = value[1]
# else:
# data[record_data["id"]][field] = value

Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

This change will cause a critical bug. The data returned by this method is serialized to JSON in the controller using json.dumps() (see ir_http.py:45). Python tuples are not JSON serializable, which will cause a TypeError at runtime when many2one fields are included in the response.

Additionally, this is a breaking API change. The existing tests expect many2one fields to return just the name string (see test_oauth_provider_scope.py:222 which expects "country_id": country.name), not a tuple.

The previous behavior of extracting only the name from the tuple was correct and should be kept.

Suggested change
# m2o filds return a tuple (id, name), now we want keep it
data[record_data["id"]][field] = value
# if isinstance(value, tuple):
# # Return only the name for a many2one
# data[record_data["id"]][field] = value[1]
# else:
# data[record_data["id"]][field] = value
# m2o fields return a tuple (id, name); return only the name
if isinstance(value, tuple):
# Return only the name for a many2one
data[record_data["id"]][field] = value[1]
else:
data[record_data["id"]][field] = value

Copilot uses AI. Check for mistakes.
data[record_data["id"]][field] = value[1]
else:
data[record_data["id"]][field] = value
# m2o filds return a tuple (id, name), now we want keep it
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

Spelling error: "filds" should be "fields".

Suggested change
# m2o filds return a tuple (id, name), now we want keep it
# m2o fields return a tuple (id, name), now we want keep it

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants