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

Implement auto import for array subtables #1302

Merged
merged 3 commits into from
Jul 4, 2023

Conversation

rishipurwar1
Copy link
Contributor

This PR fixes issue #1284. Now, Rowy can detect and suggest the schema for the array suitable.

Screen.Recording.2023-06-17.at.9.57.33.AM.mov

@vercel
Copy link

vercel bot commented Jun 17, 2023

@rishipurwar1 is attempting to deploy a commit to the Rowy Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Jun 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rowy-os ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2023 11:21am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
rowy-typedoc ⬜️ Ignored (Inspect) Jun 27, 2023 11:21am

@rishipurwar1
Copy link
Contributor Author

@harinij @shamsmosowi please review my PR.

if (tableSettings.isCollection !== false && hasData) {
if (
(tableSettings.isCollection !== false ||
tableSettings.subTableKey?.length) &&
Copy link
Member

Choose a reason for hiding this comment

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

@rishipurwar1 how is tableSettings.subTableKey?.length used here, could we just remove tableSettings.isCollection !== false part of the condition?

Copy link
Contributor Author

@rishipurwar1 rishipurwar1 Jun 17, 2023

Choose a reason for hiding this comment

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

how is tableSettings.subTableKey?.length used here

I used it to check whether an array subTable key exists or not, if it exists and hasData flag is true, then show the "import existing data" template.

could we just remove tableSettings.isCollection !== false part of the condition?

Yes, we can do that. That way, we only need to check for the hasData flag, if it's true then show the "import existing data" template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the if condition.

Copy link
Member

Choose a reason for hiding this comment

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

@iamanishroy what do you think? is there any other conditions we should watch out for?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM 👍

@harinij harinij requested a review from iamanishroy June 20, 2023 06:44
iamanishroy
iamanishroy approved these changes Jun 20, 2023
@shamsmosowi shamsmosowi merged commit ab87dbe into rowyio:develop Jul 4, 2023
3 checks passed
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.

None yet

3 participants