Description
Kyle M Hall
2017-09-14 15:29:04 UTC
Hi Kyle, Do you know if someone is working on that ? Because I will be working on something similar, that is a way to choose between several "storage spaces", eventually a subdirectory in that storage space, and other options like keeping the original name and using another URL to access those files. But I think that the configuration of those storage spaces should not happen in authorised values, but on the server ($KOHA_CONF probably) Created attachment 71017 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t I tried to test, but the patch needs a rebase. Created attachment 78620 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Patch rebased on master Pretty sure this is a duplicate of bug 20208 which is a bit fresher than mine. Closing as a duplicate on that assumption. *** This bug has been marked as a duplicate of bug 20208 *** (In reply to Kyle M Hall from comment #6) > Pretty sure this is a duplicate of bug 20208 which is a bit fresher than > mine. Closing as a duplicate on that assumption. > > *** This bug has been marked as a duplicate of bug 20208 *** Allow me to reopen it. They are both duplicates, that is true, but both have patches that solve the problem in different ways. I think at least someone from the QA team must look at both solutions and pick one. Created attachment 89219 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Doesn't apply, to start. But, after clearing conflicts we still saw a few issues: - when opening the Upload plugin popup, we always get a "no results found" message to start, even though we haven't yet done a search - we're always given the option of [root] as a directory - setting hash filename to 0 in the config did not prevent the file name from being hashed in the url Created attachment 118421 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Processing files before patches Constant subroutine Koha::Storage::KOHA_UPLOAD redefined at /usr/share/perl/5.24/constant.pm line 171. An error occurred : Inconsistent hierarchy during C3 merge of class 'Koha::UploadedFiles': current merge results [ Koha::UploadedFiles, ] merging failed on 'Koha::Objects' at /app/qatools/koha-qa.pl line 105. Created attachment 120101 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Created attachment 120102 [details] [review] Bug 19318: DO NOT PUSH - SCHEMA CHANGES Created attachment 127584 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Created attachment 127585 [details] [review] Bug 19318: Normalize uploaded file name The uploaded file name can appear in the MARC record (if using the upload plugin), and all the strings in the MARC record are normalized using Unicode::Normalize::NFC. So the file name must be normalized everywhere (in the filesystem and in the uploaded_files table too). Test plan: 1. Create a file whose name contains combining characters. For instance: echo 'contents' > $(perl -e 'print "cine\x{cc}\x{81}ma"') 0xcc 0x81 is utf8 for "Combining acute accent" (́ ) https://unicode-table.com/fr/0301/ Once normalized it should be 0xc3 0xa9 (é) 2. Use the upload tool to upload this file 3. Verify on your filesystem that the filename is normalized % ls cin* | xxd 00000000: 6369 6ec3 a96d 610a cin..ma. ^^ ^^ 4. If you have configured the storage with a `baseurl`, use the upload cataloguing plugin, and verify that the generated URL works correctly Created attachment 128844 [details] [review] Bug 19318: DO NOT PUSH - SCHEMA CHANGES Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 128845 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 128846 [details] [review] Bug 19318: Normalize uploaded file name The uploaded file name can appear in the MARC record (if using the upload plugin), and all the strings in the MARC record are normalized using Unicode::Normalize::NFC. So the file name must be normalized everywhere (in the filesystem and in the uploaded_files table too). Test plan: 1. Create a file whose name contains combining characters. For instance: echo 'contents' > $(perl -e 'print "cine\x{cc}\x{81}ma"') 0xcc 0x81 is utf8 for "Combining acute accent" (́ ) https://unicode-table.com/fr/0301/ Once normalized it should be 0xc3 0xa9 (é) 2. Use the upload tool to upload this file 3. Verify on your filesystem that the filename is normalized % ls cin* | xxd 00000000: 6369 6ec3 a96d 610a cin..ma. ^^ ^^ 4. If you have configured the storage with a `baseurl`, use the upload cataloguing plugin, and verify that the generated URL works correctly Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 128847 [details] [review] Bug 19318: (follow-up) remove uploadcategory column from tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Is there room for different storage backends here? (In reply to Tomás Cohen Arazi from comment #20) > Is there room for different storage backends here? Yes, this patch provides only 'Directory' but see Koha::Storage for documentation on defining in koha conf and provide another Adapter *** Bug 20208 has been marked as a duplicate of this bug. *** Can I have a quick rebase please? Created attachment 133772 [details] [review] Bug 19318: DO NOT PUSH - SCHEMA CHANGES Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 133773 [details] [review] Bug 19318: Allow multiple storage spaces for file uploads This patch allow to configure multiple storage spaces to be used when uploading a file. Configuration is done in $KOHA_CONF. Element 'upload_path' is replaced by 'storage', which is repeatable and can contain the following elements: - name: The storage's identifier (also used for display) - adapter: The storage's adapter is a Perl module who is responsible for actually writing in the storage (creating/deleting files/directories). There is actually only one adapter, 'directory', which writes directly on the filesystem. - adapter_params: Additional parameters required for the storage's adapter - hash_filename: Whether to prepend hashvalue to the filename or not - temporary: Whether the storage is temporary or not - baseurl: Base URL of the storage, if one doesn't want to (or can't) use opac-retrieve-file.pl This is all documented in Koha::Storage There is two built-in storages: - 'TMP', the default temporary storage (not configurable) - 'DEFAULT', the default persistent storage (configurable in $KOHA_CONF) Note that if $KOHA_CONF is not updated, uploads should continues to work (the 'DEFAULT' storage will use 'upload_path', if set) This patch affects the following pages: - Tools › Upload - Tools › Upload local cover image (ZIP file upload) - Tools › Stage MARC records for import - Tools › Upload patron images - Circulation › Offline circulation file upload - Cataloguing plugin upload.pl Test plan: 0. Before applying the patch, be sure to use the pages mentioned above and upload some files, those files should continue to be accessible after the patch 1. Apply the patch, run updatedatabase and update_dbix_class_files 2. Without modifying $KOHA_CONF, verify that files uploaded in step 0 are still accessible (you can find them in tools/upload.pl) 3. Verify that you can still upload files using the pages mentioned above 4. Edit your $KOHA_CONF, copy the <storage> element from etc/koha-conf.xml in the source, set the <path> using the value of <upload_path> and remove <upload_path> 5. Verify that you can still upload files 6. Add another storage in $KOHA_CONF, set <hash_filename> to 0, and a <baseurl>. Now try to use the cataloguing plugin and see how the generated URL change. Also note that the files keep their original filename (without the hashvalue prepended) 7. Create some subdirectories under the main storage's path (using a shell). Try to upload a new files and see that these directories appear in a dropdown list. Select one, finish the upload and verify that the file has been uploaded to the selected subdirectory. 8. Run prove t/ImportBatch.t t/Koha/Storage.t t/db_dependent/ImportBatch.t t/db_dependent/Upload.t Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 133774 [details] [review] Bug 19318: Normalize uploaded file name The uploaded file name can appear in the MARC record (if using the upload plugin), and all the strings in the MARC record are normalized using Unicode::Normalize::NFC. So the file name must be normalized everywhere (in the filesystem and in the uploaded_files table too). Test plan: 1. Create a file whose name contains combining characters. For instance: echo 'contents' > $(perl -e 'print "cine\x{cc}\x{81}ma"') 0xcc 0x81 is utf8 for "Combining acute accent" (́ ) https://unicode-table.com/fr/0301/ Once normalized it should be 0xc3 0xa9 (é) 2. Use the upload tool to upload this file 3. Verify on your filesystem that the filename is normalized % ls cin* | xxd 00000000: 6369 6ec3 a96d 610a cin..ma. ^^ ^^ 4. If you have configured the storage with a `baseurl`, use the upload cataloguing plugin, and verify that the generated URL works correctly Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 133775 [details] [review] Bug 19318: (follow-up) remove uploadcategory column from tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Tomás Cohen Arazi from comment #23) > Can I have a quick rebase please? Here you go ;) uploadcategorycode still exists in the browse block in tools/upload.pl Must admit, I'm not all that close to this feature.. why do we remove the category browser as part of this development? (In reply to Martin Renvoize from comment #29) > why do we remove the category browser as part of this development? I thought it was not useful anymore, since the main purpose of the category was to indicate a sub-directory where to put uploaded files, and with this patch it's possible without it. Can you describe a use case that is possible today with the category that is not possible anymore with these patches ? Would be glad to have a look later this week Stepping out of this one, I'm not close enough to the feature set to properly review. Thanks for offering to take it on later this week Marcel :) How do we transition from categories to the new way of handling things? We can assume that a lot of libraries are using the upload file feature. (In reply to Katrin Fischer from comment #33) > How do we transition from categories to the new way of handling things? We > can assume that a lot of libraries are using the upload file feature. There may be something missing here. I'd say we should prepend the category name to the file path or something like that (patches don't do that yet). But first we must decide if we need to keep upload category or not. Just at first glance, I do not understand why we need so much changes to accomplish what you can do already. You can add categories or subdirectories via an AV (which could be moved away to its own config). Probably you dont want to put uploads everywere on your server. So one paraplu directory just as upload_path should be fine? I have decided to move this to 'In discussion' for now. My reasoning: The patch set does appear to do much more than is written on the 'tin': It claims to allow for custom upload paths, but it does a lot more than that. There are unresolved questions about some of the design decisions and how we can deal with the consequences: * While Kyle had originally proposed using an AV in the initial description, this patch set moves everything away from the GUI into the koha-conf.xml. In doing so, it removes the UPLOAD AV. If this is proven to be the right path, it would require more work to remove the AV category properly and ensure a clean upgrade path (see comment#34, comment#29). But we also should discuss if this is a required step as it takes away control from libraries that often don't have easy access to server side configuration. Personally this feels contrary to what I see in other work (like making a GUI for SMTP servers). * Adds other extra features, like: > hash_filename: Whether to prepend hashvalue to the filename or not. I wonder if some of these features would be better suited as follow-ups to keep the initial bug smaller. (comment#35) My feeling is that these conceptual questions cannot be resolved in QA, but need to be resolved before it reaches the QA step with a clear test plan/upgrade path/reasoning of changes. |