Skip to content
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

fix: save&connect COMPASS-8068 #6037

Merged
merged 10 commits into from
Jul 19, 2024
Merged

fix: save&connect COMPASS-8068 #6037

merged 10 commits into from
Jul 19, 2024

Conversation

paula-stacho
Copy link
Contributor

@paula-stacho paula-stacho commented Jul 17, 2024

Description

Aside of renaming the button to save&connect, this PR is trying to make a clearer distinction between 'connect' and 'saveAndConnect', legacy and new code. For example making sure the submitForm is always explicitly one or the other - previously we had inconsistent behaviour between submit via button and submit via keystroke, likely because we didn't notice the other calls. So after I removed the extra "save", we were saving only once in the first case but zero times in the second! 🤦 I blame the complexity of the legacy form, and can't wait for the cleanup 😁 🔥

Also I marked ConnectFormActions as legacy and replaced it's unit tests with tests for the new ConnectionFormModalActions. Today I learned these are still used for VSCode, but I don't think that's a reason to maintain the separate component - we just need some flags to prepare the new component for VSCode. I created COMPASS-8098 to be done before we do the clean up and enable the new form in VSCode (COMPASS-7762)

Scenarios tested:

  • edit (submitted with btn or enter)
  • duplicate (submitted with btn or enter)
  • create (submitted with btn or enter)

Checklist

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)

@github-actions github-actions bot added the fix label Jul 17, 2024
@paula-stacho paula-stacho added the feature flagged PRs labeled with this label will not be included in the release notes of the next release label Jul 18, 2024
@paula-stacho paula-stacho marked this pull request as ready for review July 18, 2024 07:41
is_favorite: Boolean(favorite),
is_recent: Boolean(lastUsed && !favorite),
is_favorite: isFavorite,
is_recent: Boolean(lastUsed && !isFavorite),
Copy link
Contributor Author

@paula-stacho paula-stacho Jul 18, 2024

Choose a reason for hiding this comment

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

this is a driveby - noticed via a broken e2e test. the favorite object contains the color and name, which can now exist on non-favorite connections 🙈 . we didn't rename because it would require a migration of the stored connections. the proper way to check for a favorite is via savedConnectionType.

Copy link
Contributor

Choose a reason for hiding this comment

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

had to update another unit test: a192470

@lerouxb lerouxb merged commit b3b8994 into main Jul 19, 2024
27 of 28 checks passed
@lerouxb lerouxb deleted the COMPASS-8068-2 branch July 19, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature flagged PRs labeled with this label will not be included in the release notes of the next release fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants