Skip to content

Conversation

@mabaasit
Copy link
Collaborator

@mabaasit mabaasit commented Oct 29, 2025

Allow users to edit field type on diagram node when a field is double clicked (like field name).

Preview
Editable.Field.mov

Description

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@mabaasit mabaasit force-pushed the COMPASS-9799-field-type-on-node branch from 278cbbf to 48f4e93 Compare November 17, 2025 12:02
@mabaasit mabaasit changed the title WIP: feat(data-modeling): allow inline field type changes COMPASS-9799 feat(data-modeling): allow inline field type changes COMPASS-9799 Nov 17, 2025
@mabaasit mabaasit force-pushed the COMPASS-9799-field-type-on-node branch from 55dde32 to 56d6196 Compare December 11, 2025 11:09
@github-actions github-actions bot added the feat label Dec 11, 2025
@mabaasit mabaasit marked this pull request as ready for review December 11, 2025 11:14
@mabaasit mabaasit requested a review from a team as a code owner December 11, 2025 11:14
Copy link
Contributor

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 PR enables inline editing of field types in diagram nodes when double-clicked, similar to the existing field name editing capability. The change centralizes the derivation of oldTypes within the changeFieldType function rather than requiring callers to provide it, improving the API design and reducing redundancy.

Key changes:

  • Refactored changeFieldType to derive oldTypes internally from the schema
  • Extracted BSON types into a shared utility module (FIELD_TYPES)
  • Connected field type change handler to diagram component

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/compass-data-modeling/src/utils/field-types.ts New utility module exporting FIELD_TYPES constant for BSON type enumeration
packages/compass-data-modeling/src/store/diagram.ts Refactored changeFieldType to derive oldTypes internally, removing it from the function signature
packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx Updated to use shared FIELD_TYPES constant and removed oldTypes parameter from changeFieldType call
packages/compass-data-modeling/src/components/diagram-editor.tsx Added onChangeFieldType handler to enable inline field type editing in diagram nodes
packages/compass-components/package.json Bumped @mongodb-js/diagramming to ^2.2.2 to support the new inline editing functionality

Comment on lines +867 to 868
const oldTypes = field?.fieldTypes;
if (!field) throw new Error('Field not found in schema');
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The oldTypes is assigned from field?.fieldTypes before checking if field exists. If field is undefined, oldTypes will be undefined, which could cause issues in telemetry or undo logic. Move this assignment after the null check on line 868.

Suggested change
const oldTypes = field?.fieldTypes;
if (!field) throw new Error('Field not found in schema');
if (!field) throw new Error('Field not found in schema');
const oldTypes = field.fieldTypes;

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤦

onNodeExpandToggle: isCollapseFlagEnabled
? handleNodeExpandedToggle
: undefined,
fieldTypes: FIELD_TYPES,
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The new fieldTypes prop being passed to the diagram is not covered by tests. Consider adding test coverage to verify that the field types are correctly passed and rendered in the diagram component.

Copilot uses AI. Check for mistakes.
@codeowners-service-app
Copy link

Assigned gagik for team compass-developers because gribnoysup is out of office.

@paula-stacho paula-stacho self-requested a review December 11, 2025 15:07
@paula-stacho
Copy link
Collaborator

paula-stacho commented Dec 11, 2025

Somehow the change of a field type in diagram doesn't update the drawer. this works for field name but I don't remember if we had to do something for it, I would have expected this to work out of the box 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants