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

Instance importer/exporter #1561

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

cayb0rg
Copy link
Contributor

@cayb0rg cayb0rg commented Jan 30, 2024

This PR adds an instance importer/exporter to a few components:

Second Iteration

Screenshot 2024-04-04 at 1 01 20 PM

Save History:

Screenshot 2024-04-04 at 1 03 59 PM

User Managed Instances/Admin Instance Search:

Screenshot 2024-04-04 at 1 04 46 PM

Toast:

image

First Iteration (Meatball Menu)

My Widgets Page:

image

Details

Media exporting works for new and updated widgets only and will not appear for widgets with missing entries in the map_asset_to_object table. This table was not being properly updated in Widget_Question::db_store (NEEDS CHECK ON PROD).

Details:

  • Adds new API endpoint for updating a widget's qset
  • Adds new API endpoint for fetching assets by instance ID or qset ID, with option to get media from all qsets or a specific qset
  • Adds a generalized "UseExportType" hook for exporting the qset, instance, media, or all of the above
    • Qset/Instance use API endpoints, while media uses direct download
  • Added new API endpoint for importing a widget instance with validation, including asset URL validation
  • Import is available in My Widgets and Save History dialog
  • Export is available in My Widgets, Save History, Admin Instance Search, and Admin User managed instances
  • Adds a "toast" hook for success/error messages
  • Updates apiGetQuestionSet and apiGetWidgetInstance to use object syntax for parameters

While I don't see a use case for importing media in Materia itself (since we're just pointing at already existing assets), this is something that would most likely need to be added in the Materia Community Library.

To do:

  • Fix import instance button styling?
  • Reduce/reword export option menu items?
  • Find a way to use a POST request instead of a GET to get the media ZIP file for better error handling? I attempted this here: 3980454, but couldn't get it to work.
  • Certain widgets store media assets in other locations such as Labeling. Add these assets to the map_asset_to_object table as well through a recursive search

@clpetersonucf clpetersonucf deleted the branch ucfopen:master February 9, 2024 19:57
@clpetersonucf clpetersonucf reopened this Feb 9, 2024
@clpetersonucf clpetersonucf changed the base branch from dev/10.1.0 to master February 9, 2024 20:01
@cayb0rg cayb0rg marked this pull request as ready for review February 12, 2024 16:48
@clpetersonucf
Copy link
Member

Overall, awesome start on this feature 👍 Some initial thoughts:

  • I hadn't considered splitting import/export into both instances and qsets, my first thought is there may not be enough of a distinction to lay users, but now that you've fleshed out that functionality, I could see making use of it. My one concern is exporting qsets that contain media assets, since those assets aren't included and assume the qset will be installed in the same system (?)
  • I do think the way export options are displayed in My Widgets can be improved. We don't really expose "instances" and "qsets" as terminology anywhere else, so there may be confusion with just having those options available without any kind of explanation. What about replacing the 3-button menu (which looks great, btw) with a generic "Export" icon, which provides an intermediate modal explaining the two options and their purpose?
  • The Import Instance option is kinda hidden down there at the bottom of the instance list, but I think that's okay - it's not likely to be a widely-used feature, so I'm okay making it a little obfuscated.
  • The creator Save History dialog is missing the close/cancel option, and it might benefit from being renamed to something like "Manage Saves" if it's going to be extended to handle import/export as well.
  • The toasts are awesome, I love them! However I did notice that they need a bigger z-index value as they tend to hide between certain other UI elements. Maybe they're just shy.

@clpetersonucf clpetersonucf changed the base branch from master to dev/10.2.0 March 27, 2024 19:03
@cayb0rg
Copy link
Contributor Author

cayb0rg commented Apr 2, 2024

My one concern is exporting qsets that contain media assets, since those assets aren't included and assume the qset will be installed in the same system (?)

That's a good point. "Export Qset" simply exports the qset with no assets attached while "Export All" will. I think this could be explained in the intermediate modal. Even so, uploading assets to a new system would likely change the IDs, rendering the qset image ids useless. I would like to enable instance+media transfer between systems, so I'll look into this.

We don't really expose "instances" and "qsets" as terminology anywhere else, so there may be confusion with just having those options available without any kind of explanation. What about replacing the 3-button menu (which looks great, btw) with a generic "Export" icon, which provides an intermediate modal explaining the two options and their purpose?

I was worried about the wording here as well. I like that idea and think it will clarify things for the user.

The Import Instance option is kinda hidden down there at the bottom of the instance list, but I think that's okay - it's not likely to be a widely-used feature, so I'm okay making it a little obfuscated.

If you think I should make it a different color (like the orange) to make it stand out more, I can do that. My intention here was to make it slightly more hidden since it's not widely-used as you said, and I see it as more of an advanced option.

The creator Save History dialog is missing the close/cancel option, and it might benefit from being renamed to something like "Manage Saves" if it's going to be extended to handle import/export as well.

👍🏻

The toasts are awesome, I love them! However I did notice that they need a bigger z-index value as they tend to hide between certain other UI elements. Maybe they're just shy.

Haha, good catch!

Also, sorry for taking so long to respond.

@clpetersonucf clpetersonucf changed the base branch from dev/10.2.0 to dev/10.3.0 July 16, 2024 19:19
@clpetersonucf clpetersonucf deleted the branch ucfopen:master December 12, 2024 14:42
@clpetersonucf clpetersonucf reopened this Dec 12, 2024
@clpetersonucf clpetersonucf changed the base branch from dev/10.3.0 to master December 12, 2024 14:48
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.

2 participants