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 - user must be of type string #13688

Merged
merged 11 commits into from
May 15, 2024
Merged

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented May 14, 2024

Description

Some old deprecated user columns had some constrains, forcing the data to be strings. With the recent changes, we were storing them as simple arrays, and this is conflicting with its constrains.

Addresses

Launchcontrol

Fixing a bug causing save issues with old deprecated single columns

Copy link

linear bot commented May 14, 2024

@adrinr adrinr marked this pull request as ready for review May 14, 2024 16:09
@adrinr adrinr requested a review from a team as a code owner May 14, 2024 16:09
@adrinr adrinr requested review from samwho and mike12345567 and removed request for a team May 14, 2024 16:09
@@ -80,7 +80,7 @@ export async function search(

const table = await sdk.tables.getTable(options.tableId)
options = searchInputMapping(table, options, {
isSql: !!table.sql || !!env.SQS_SEARCH_ENABLE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the removal of this?

Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

LGTM

@mike12345567
Copy link
Collaborator

Addresses: #13684

@adrinr adrinr marked this pull request as draft May 15, 2024 09:01
@adrinr adrinr marked this pull request as ready for review May 15, 2024 09:47
@adrinr adrinr requested a review from mike12345567 May 15, 2024 10:18
Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

LGTM - nice improvements, making the bulk import API better!

@adrinr
Copy link
Collaborator Author

adrinr commented May 15, 2024

During these changes, we realised that bulk import (used for tests) was not treating the input row with functions like bbprocessor. This was causing some tests to not work as expected

@adrinr adrinr enabled auto-merge May 15, 2024 10:23
@adrinr adrinr merged commit 9f2f177 into master May 15, 2024
10 checks passed
@adrinr adrinr deleted the budi-8251-user-must-be-of-type-string branch May 15, 2024 10:27
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants