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

Bulk Upload #91

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

Bulk Upload #91

wants to merge 113 commits into from

Conversation

davegson
Copy link

@davegson davegson commented Mar 24, 2025

About the Code Structure:

  • we have 4 steps to go through for a bulk upload with 4 routes

    1. Load CSV: '/bulk-upload/csv-config'
    2. Data Mapping: '/bulk-upload/meta-data-config'
    3. Preview: '/bulk-upload/preview'
    4. Upload: '/bulk-upload/upload'
  • when only /bulk-upload is called, the index redirects the user to their last step via middleware

  • the main config is in field-settings.js, where as much logic is derived from as possible. The idea is that you can configure that file and through that you can change or add more fields. I think there are still places where we can export its custom logic into the field-settings, but so you understand the overall goal. Especially since there is lots of custom logic for each phaidra component - this was the attempt of unifying all that logic.

  • another central place of logic is store/bulk-upload.js, which defines all logic for Local Storage

  • all other files basically reference to those two main logic files but also have individual logic where needed

that's it for the intro - looking forward to hear from your experienced perspective!

- data is stored in store/bulk-upload.js
- basic UI elements
- closing and revisiting page works, as it grabs previously set data from store
- better UX
- also prevents double mapping of same column
more consistent than sometimes using it and sometimes using direct $store
- change that you can select CSV or a defined phaidra element
instead of having a form for each field, just load the phaidra element inputs - the vue will then take care of loading the appropriate values
since ; splits the columns, it must be different than ;
use vuex state instead of old store solutions which no longer worked

also cleaned up a lot of the code to be much simpler
now each field both have a source and a value, which makes it easier to differentiate between csv source and phaidra fields
davegson added 28 commits March 6, 2025 12:44
simplifies logic & removes unneeded code
the method getSubFieldMapping only is triggered in multi-fields, so no need for those if elses
first row in csv are headers an should not be displayed as a data row
in order to also show optional phaidra sub-fields and not default source from phaidra even if unsourced
made logic a lot simpler, less if else trees
this will make it easier to re-use
the setting of columns was unnecessary since it can be derived from the csvContent

so just define a getter in bulk-upload.js and use that wherever headers are needed
also make it hard to tinker around with stuff more. Disable back & upload button, overlay upload form
left overs from another logic structure
try to get as much info from field-settings page as possible, making the logic in upload less complex
reduces file size of upload.vue
instead of havig a method simply accessing data through getters, just get the values directly. Simpler. Better. Cleaner.
copied from Rasta's uploader implementation. Some phaidra fields need this in order to properly set data
csv sources also need to structure their logic off of field-settings as the data - this moves further logic into field-settings (the main source of phaidra field logic)
this way we can easily customize what needs to be done for every csv field. EG: we split keywords by ',' so an array is passed to API. Which needs that format to correctly create the keywords
@davegson davegson requested a review from RastislavHudak March 24, 2025 10:07
@davegson davegson self-assigned this Mar 24, 2025
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.

1 participant