geodesicsolutions-community / geocore-community

GeoCore Community, open source classifieds and auctions software
MIT License
9 stars 6 forks source link

Cleanup bulk uploader sample data #148

Open vicos59 opened 2 years ago

vicos59 commented 2 years ago

fixes #18

also referenced in #120

-- geodesicsolutions.com-->geodesicsolutions.org -- Emails all go to @noreply.com now -- Downloaded free to use sample images from pixabay.com. Files are included in a folder bulk_uploader/docs/samplepics with a README. Probably best keep everything self-contained as far as the distro.

I guess the images also need to go on:

http://www.geodesicsolutions.org/demos/photos/

unless you have a different plan.

jonyo commented 2 years ago

I guess the images also need to go on:

http://www.geodesicsolutions.org/demos/photos/

No need for the longer path, lets call it what it is, something like bulk-uploader-sample-images/ or similar. I'd say we should add it there first, if you do a PR then I can review/merge it. Once it is on the website you can test the sample CSV import files to make sure the import works as expected using the updated url.

vicos59 commented 2 years ago

do a PR then I can review/merge it

Done!

https://github.com/geodesicsolutions-community/website/pull/17/files

jonyo commented 2 years ago

@vicos59 the images are merged in the website and updated, be sure to test the bulk upload with the sample CSV and let me know if it works now.

vicos59 commented 2 years ago

example_upload_auctions.txt fails at the final step:

http://localhost:8081/admin/index.php?page=addon_bulk_uploader_main_config&p=3

TypeError: array_merge(): Argument #1 must be of type array, null given in /var/www/html/src/addons/bulk_uploader/admin.php:2007
Stack trace:
#0 /var/www/html/src/addons/bulk_uploader/admin.php(2007): array_merge(NULL, Array)
#1 /var/www/html/src/addons/bulk_uploader/admin.php(1915): addon_bulk_uploader_admin->alterUploadForm()
#2 /var/www/html/src/addons/bulk_uploader/admin.php(416): addon_bulk_uploader_admin->displayBulkUploader()
#3 /var/www/html/src/admin/php5_classes/Admin.class.php(543): addon_bulk_uploader_admin->display_addon_bulk_uploader_main_config()
#4 /var/www/html/src/admin/index.php(16): geoAdmin->load_page()
#5 {main}
vicos59 commented 2 years ago

P.S. I know I need to mod .gitignore to exclude files from the 2 new folders. I am doing this last or else it makes a mess on Windows. Will remove the temp files before final commit.

P.S. the temp file in uploads/ now is the sample data that was uploaded and was processing when the fatal happened.

jonyo commented 2 years ago

By the way, I don't think you need a readme in every empty folder... If there is a specific reason to sure, just mentioning it isn't like a requirement or anything, like there is none in user_images or templates_c etc.

vicos59 commented 2 years ago

I am a long time practitioner of self-documenting code, it makes life much easier for me and the next guy who has to make changes x+ years from now.

I see the README.md as essential in those folders because user-generated data is being created/stored in a place where normally there would only be code. In my apps, I typically put data files in a data/ folder parallel to webroot/ where they can never be directly accessed by a URL. It helps me keep the code separated from the data. That's just the way I have always done it.

jonyo commented 2 years ago

In my apps, I typically put data files in a data/ folder parallel to webroot/ where they can never be directly accessed by a URL.

That is actually one of the things I was hoping to improve in a future version... Not the initial release. But it would be nice to have better separation. Like code would go in src, web accessible in webroot, etc. like you said. I'm used to cakephp so one option might be to use it to handle the framework, maybe finally get rid of that huge switch in the index.php 😆 . Though I think we would need a lot more people using it to warrant doing a refactor of that size.