-
Notifications
You must be signed in to change notification settings - Fork 247
chore(data-modeling): update node name to be collection name #7102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…espace, update test to be timezone independent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Updates the display title for diagram nodes to use the collection name rather than the full namespace and tweaks a test’s timestamps for timezone independence.
- Switch node title from
nsto the parsedcollectionviatoNS - Import
toNSutility in the diagram editor - Adjust test timestamps to avoid timezone-related failures
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/compass-data-modeling/src/components/diagram-editor.tsx | Use toNS(coll.ns).collection for node titles |
| packages/compass-data-modeling/src/components/diagram-card.spec.tsx | Modify timestamp fixtures for timezone independence |
Comments suppressed due to low confidence (1)
packages/compass-data-modeling/src/components/diagram-editor.tsx:239
- Consider adding a unit or integration test to assert that the node’s title renders the collection name (from
toNS) instead of the full namespace, ensuring future changes don’t regress this behavior.
title: toNS(coll.ns).collection,
| createdAt: '2023-10-01T00:00:00.000Z', | ||
| updatedAt: '2023-10-03T00:00:00.000Z', | ||
| createdAt: '2021-10-01T00:00:00.000Z', | ||
| updatedAt: '2023-10-03T00:00:00.000', |
Copilot
AI
Jul 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ISO timestamp is missing the Z timezone designator, which may cause inconsistent parsing across environments; consider restoring the trailing Z for a fully qualified UTC time string.
| updatedAt: '2023-10-03T00:00:00.000', | |
| updatedAt: '2023-10-03T00:00:00.000Z', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We render it based on the user's time zone so the change here is making it less brittle.
We could change what we're looking for in the check to be on of two days if we want, or something else entirely, I don't think it's worth spending too much time on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting.. maybe we could fix the timezone for the tests? So that we don't need to think about this for future tests, since it's apparently easy to miss for us Europeans 😅
I couldn't find a mocha setting but we could change the npm scripts to: "test": "TZ=UTC mocha",.
re https://mongodb.slack.com/archives/C08P2P6ADJL/p1751386769660719
Drive by updated a test to be time zone independent (it failed on my local, maybe it passes in the EU?)