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

Improved handling attempted access of deleted files in importer #15112

Merged
merged 22 commits into from
Aug 20, 2024

Conversation

marcusmoore
Copy link
Collaborator

@marcusmoore marcusmoore commented Jul 17, 2024

Description

The Issue...

Previously, attempting to interact with a file that was deleted would fail hard. Here's an example of deleting an import file in one tab and then interacting with it in another to demonstrate:

CleanShot.2024-07-18.at.11.41.55.2.mp4

The Fix...

With this PR, an error message is shown when attempting to select the deleted import file:

CleanShot.2024-07-18.at.11.44.00.mp4

The Details...

To support this I made overall improvements to the Import component. Generally speaking:

  • Cleaned up mount and render methods
  • Implemented two computed properties for the collection of files and the currently selected file
    • And since we're no longer binding to the Import eloquent model we're closer to switching back to the default for Livewire 3 and turning off legacy_model_binding

Not Quite There...

But...having already "opened" the deleted import and attempting to interact with it doesn't fail hard anymore but also doesn't show an error message:

CleanShot.2024-07-18.at.11.44.33.mp4

I attempted to use the exception hook to catch the ModelNotFound exception but it doesn't seem to work for computed properties as mentioned in this discussion. I went down the path of making an overwritten Computed property but wasn't able to write it up correctly. I think we can wait until Livewire implements the fix itself and we (should) get it for free after updating. What do you think @snipe?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Copy link

what-the-diff bot commented Jul 17, 2024

PR Summary

  • Enhancements to the Importer Component

    • The Importer's code was reorganized, improving the layout and including new features.
    • Introduction of a 'progress bar' feature was made to provide visual feedback during file imports.
    • The internal naming of the active import file was clarified, improving code readability.
    • Properties specifying the 'headerRow' and 'typeOfImport' for an import file were added, helping specify file details more explicitly.
    • The 'mount' and 'selectFile' methods were revised to better facilitate file selection during the import process.
    • A 'clearMessage' method was added, providing a way to remove displayed messages.
    • Computed properties for 'files' and 'activeFile' were introduced, creating a more effective way to retrieve and track import files.
  • Addition of Import Permission in User Factory

    • A new capability to explicitly set an 'import' permission for users was added. This offers refined control over which users can import files.
  • Updates to the Importer Blade View

    • The shorthand variable-paths were updated to improve consistency and clarity. This change enhances the readability and maintainability of the blade view code.
  • Introduction of Importer Test

    • A new test file for the Importer was created. This will facilitate regular and systematic checking of the Importer's functions, ensuring that they are working correctly and as expected.

@marcusmoore marcusmoore marked this pull request as ready for review August 19, 2024 23:36
@snipe
Copy link
Owner

snipe commented Aug 20, 2024

I think this looks great, even if it's not perfect, it's a far better UX than what we had.

@snipe snipe merged commit f8c72fb into snipe:develop Aug 20, 2024
8 of 9 checks passed
@marcusmoore marcusmoore deleted the livewire-importer-improvements branch August 20, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants