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

chore: add columns and transformedData to ArtworkImport #6451

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

mzikherman
Copy link
Contributor

I think this will allow Volt's table view of an import to be a bit generic, just showing the user the columns they initially included (and transformed/normalized data values).

Now, Volt still needs to 'know' for example that titles can have inline editing enabled and maybe should render differently than another column type, but I feel like that could be a little config within Volt: column names -> Component kinda thing. We can see if that makes sense to move somewhere upstream - into MP kinda like homeview. But, since Volt is likely to be the only client for this whole imports schema, we can probably keep that stuff in Volt.

Ultimately, we want the specifics of the CSV in question to 'inform' the Volt table rendering of the import. This is b/c different partners may start to include different columns, we have optional columns, etc.

We can even drive Volt's experience for images this way without a feature flag! If an import included image filenames as a column - it can present those and ask for the image upload step, as we currently do. But if an import didn't include that column...it won't render in the table and the UI can skip that step.

@mzikherman mzikherman requested a review from a team February 20, 2025 01:33
@mzikherman mzikherman self-assigned this Feb 20, 2025
Copy link
Contributor

@jpotts244 jpotts244 left a comment

Choose a reason for hiding this comment

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

🫡

@mzikherman mzikherman merged commit 41065bb into main Feb 20, 2025
4 checks passed
@mzikherman mzikherman deleted the add-transformed-data-to-imports branch February 20, 2025 01:44
@artsy-peril artsy-peril bot mentioned this pull request Feb 20, 2025
@Mitch-Henson
Copy link
Contributor

Mitch-Henson commented Feb 20, 2025

This all looks good to me though I wanted to comment on a line that you wrote

If an import included image filenames as a column - it can present those and ask for the image upload step, as we currently do

I think that this is an assumption that needs to be validated by Anita as this is a business decision in my mind. That being said, we can still definitely take advantage of this logic and instead do only this validation in the backend, e.g. only reading from approved columns which can be feature flagged. This will hopefully make it a lot easier to separate the functionality as we can mostly limit it to a single API call

@mzikherman
Copy link
Contributor Author

We can double-check for sure but no reason to gate the feature....if a CSV upload includes image filenames then we can unlock the advanced functionality.

@mzikherman
Copy link
Contributor Author

This just came up in retro actually - we should lean on less flags the better (IMO) - and its ok to let something be 'discoverable' as well - but we can always double-check with Core.

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.

3 participants