Add table icons to the WorkBench data sets dialog based on base table

[grantfitzsimmons]

Solves #1562

Steps to test:

1. Verify that all existing data sets use the base table icon rather than the "WB" icon in the WorkBench data sets dialog
2. Create a new data set in several base tables
3. Add many columns with complicated mappings
4. Add values across the spreadsheet
5. Save and upload
6. Open dialog and verify the base table icon is used and all looks correct
7. Upload an existing spreadsheet
8. Map, validate, and upload
9. Open dialog and verify the base table icon is used and all looks correct
10. Perform the testing steps 2-8 with the batch attachment uploader to ensure all is working as expected (no icons will show in that dialog)

[maxpatiiuk]

Code looks good but I have a concern
The reason upload plan wasn't added by Ben to this endpoint is that it could be bad for performance (make the data sets dialog take more time to load, especially since we don't have any pagination or caching for this endpoint)

could you modify this PR to only add baseTableName field instead of the entire uploadPlan?

[grantfitzsimmons]

The reason upload plan wasn't added by Ben to this endpoint is that it could be bad for performance (make the data sets dialog take more time to load, especially since we don't have any pagination or caching for this endpoint)

This is interesting– and absolutely something I considered. The upload plan is generally much smaller than the whole data set, but there are situations where it can become fairly complicated. I'll see if I can only add baseTableName

[maxpatiiuk]

This is interesting– and absolutely something I considered. The upload plan is generally much smaller than the whole data set, but there are situations where it can become fairly complicated. I'll see if I can only add baseTableName

No problem - the other thing you can do is benchmark what impact this change makes on a representative or worst case database

And also, adding pagination is something we have to do anyway at some point, so that would mitigate the concern quite a bit

[maxpatiiuk]

spoiler about the cause of the TypeScript error you are getting in GitHub Actions

The typescript type you modified appears to be used not just in the WorkBench, but also in the Attachments Bulk Upload - you would likely need to create a separate TypeScript type that extend this one with one new field added, and then use this new TypeScript type in the WorkBench

[maxpatiiuk]

as per the performance test you did, I am fine with the way this PR is (or with only including baseTableName)

btw, if you are curious, take a look at GraphQL (as opposed to REST APIs that we use) - with GraphQL, the front-end defines what data it needs, and the back-end sends just that data

[maxpatiiuk]

👍

[realVinayak]

Nice! Maybe extend that to batch image upload too? You can also get what the table is from the uploadplan there!

[emenslin]

Everything workbench wise looks good, I like the new icons. The last part of the testing checklist says to test the same things on batch attachment uploader however there weren't any changes made there.

[realVinayak]

funny that we have so multiple names for batch upload
batch attachment upload
batch image upload
bulk uploader (in the code)

[grantfitzsimmons]

Everything workbench wise looks good, I like the new icons. The last part of the testing checklist says to test the same things on batch attachment uploader however there weren't any changes made there.

I should have clarified that I did not intend for the icons to be included in the batch attachment upload tool, I just wanted to ensure no functionality was lost/broken due to these changes!

[grantfitzsimmons]

There you go– your dreams have come true @realVinayak!

[grantfitzsimmons]

I'll fix the tests soon– no merging until that's complete!

[emenslin]

Looks good in batch attachment upload now as well! :)

[bronwyncombs]

Excellent 🐸 🎩