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

Got duplicate-field mapping and minimum-mapping requirements implemented #15442

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

I think this needs some UI love, for sure, and definitely some testing. But after that, I think it should be ready to go.

This handles a few problems - the main one being where if you switch from one import type to another, the field-names didn't get properly updated. That was getting users confused where they thought we had features we didn't, when you picked import-type: Assets, and then say, oops, I meant Accessories! The field names wouldn't quite update right.

Next, once I got that working I decided to change the "QTY" option in the drop-down to "Quantity" which I think reads a little better. In working with that, I found that we were not 'auto-guessing' quantity fields quite right, so I fixed that.

While I was there, I decided to add some code to detect if you try to map two separate CSV fields to the same Snipe-IT field. That doesn't make sense, and we shouldn't allow it.

And finally, I Implemented the start of some logic to prevent you from importing if you don't map the basic number of required fields. We can probably start to expand this out as we figure out more detailed rules on what we can and cannot allow, so I went generally pretty conservatively here - the hope would be that I would rather you import and error, rather than us incorrectly disallow you from importing.

UI fixes needed:

I don't like the visual way I display the "duplicate fields detected" thing. I just put some placeholder text in there, but it should be translated and look more error-ey, and maybe even live somewhere else in the UI.

Right now there's no really clear feedback on when you haven't mapped enough fields - the submit button just goes 'disabled'. And I suspect I don't correctly disable the submit button when you have mapping duplicates - that feature didn't exist yet when I built the first part.

And I had to do a lot of logic to figure out what the minimum fields you had to map, mostly by looking at the importer itself. I'm not necessarily sure I got all of that right.

I'm also uncomfortable with the minimal_fields function thing - it looks a little gross in places, and I hate the @ signs being sprinkled anywhere. But this was already getting too big anyways.

Copy link

what-the-diff bot commented Sep 2, 2024

PR Summary

  • Inclusion of App\Models\Setting in app/Livewire/Importer.php
    The code App\Models\Setting was added to the use statements in app/Livewire/Importer.php, which aids in the smooth interaction with Setting model data within the Importer class.

  • New Public Properties
    Two new public properties named mapping_errors and enough_data_to_import were added. These would help track any errors related to mapping and ensure there is adequate data in place for the import functionality to work correctly.

  • Logging Method Updates
    The updatingTypeOfImport and updatedFieldMap methods were introduced. These methods are part of the logic that helps flag errors in mapping.

  • Adjustments to mount Method
    The mount method underwent several changes including property resets and structuring of information. These adjustments are aimed at enhancing functionality and ensuring an error-free mapping and import process.

  • Modifications in selectFile method
    This method was revised to include a reset of the mapping_errors property and engagement of the updatedFieldMap method. This ensures that mapping errors, if any, are dealt with correctly when a file is selected for importing.

  • User Interface Update in importer.blade.php
    The UI on the importer.blade.php view was updated to show the status of field mapping in a new column. Also, it has been programmed to disable the import button if required data for import is insufficient.

Note: No changes have been reflected in resources/views/livewire/importer.blade.php.

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.

1 participant