ENH Use archive text when file archiving is enabled #1378

emteknetnz · 2023-08-08T00:23:09Z

Issue silverstripe/silverstripe-versioned-admin#300

AssetAdmin::getClientConfig() is an array of PHP values that are made available in JS

This pull-request is NOT updating jest tests, instead they will be updated in #1379 instead

I'm doing this because I want to test silverstripe/gha-merge-up#6 going cross major, though jest tests will definately cause merge-conflicts due to the jest tests being rewritten in 2 #1355

After merging - manually run the merge-up action on asset-admin using the default branch - https://github.com/silverstripe/silverstripe-asset-admin/actions/workflows/merge-up.yml

emteknetnz · 2023-08-08T01:15:02Z

client/lang/src/en.json

@@ -3,16 +3,31 @@
    "AssetAdmin.BACK": "Back",
    "AssetAdmin.BACK_DESCRIPTION": "Navigate up a level",
    "AssetAdmin.BULK_ACTIONS_CONFIRM": "Are you sure you want to %s these files?",
+    "AssetAdmin.BULK_ACTIONS_ARCHIVE": "Archive",
+    "AssetAdmin.BULK_ACTIONS_ARCHIVE_ITEMS_CONFIRM": "You're about to archive %s file(s) which may be used in your site's content. Carefully check the file usage on the files before archiving the folder.",


This message is knowingly wrong where it says archiving the folder instead of archiving the file(s) - this is because this text is tested in a jest test that will be updated in #1379

emteknetnz · 2023-08-08T01:15:11Z

client/lang/src/en.json

    "AssetAdmin.BULK_ACTIONS_DELETE_FOLDER": "These folders contain files which are currently in use, you must move or delete their contents before you can delete the folder.",
+    "AssetAdmin.BULK_ACTIONS_DELETE_FOLDER_CONFIRM": "Are you sure you want to delete this folder?",
+    "AssetAdmin.BULK_ACTIONS_DELETE_FOLDERS_CONFIRM": "Are you sure you want to delete these folders?",
+    "AssetAdmin.BULK_ACTIONS_DELETE_ITEMS_CONFIRM": "You're about to delete %s file(s) which may be used in your site's content. Carefully check the file usage on the files before deleting the folder.",


This message is knowingly wrong where it says deleting the folder instead of deleting the file(s) - this is because this text is tested in a jest test that will be updated in #1379

emteknetnz · 2023-08-08T01:15:19Z

client/src/containers/AssetAdmin/AssetAdmin.js

-              i18n._t(
-                'AssetAdmin.BULK_ACTIONS_DELETE_FAIL',
-                '%s folders/files were successfully deleted, but %s files were not able to be deleted.'
-              ),


This key was wrong because in en.json the text is archived instead of deleted

emteknetnz · 2023-08-08T01:18:56Z

client/src/containers/BulkDeleteConfirmation/BulkDeleteMessage.js

+    let transKey = 'AssetAdmin.BULK_ACTIONS_DELETE_ITEMS_CONFIRM';
+    let transDefault = [
+      "You're about to delete %s file(s) which may be used in your site's content.",
+      'Carefully check the file usage on the files before deleting the folder.'


This message is knowingly wrong where it says deleting the folder instead of deleting the file(s) - this is because this text is tested in a jest test that will be updated in #1379

emteknetnz · 2023-08-08T01:19:11Z

client/src/containers/BulkDeleteConfirmation/BulkDeleteMessage.js

+      transKey = 'AssetAdmin.BULK_ACTIONS_ARCHIVE_ITEMS_CONFIRM';
+      transDefault = [
+        "You're about to archive %s file(s) which may be used in your site's content.",
+        'Carefully check the file usage on the files before archiving the folder.'


This message is knowingly wrong where it says archiving the folder instead of archiving the file(s) - this is because this text is tested in a jest test that will be updated in #1379

emteknetnz · 2023-08-08T01:53:53Z

client/src/containers/AssetAdmin/AssetAdmin.js

          this.props.actions.toasts.success(
            i18n.sprintf(
-              i18n._t('AssetAdmin.BULK_ACTIONS_DELETE_SUCCESS', '%s folders/files were successfully deleted.'),


This key was wrong because in en.json the text is archived instead of deleted

emteknetnz · 2023-08-08T01:54:55Z

client/src/containers/BulkDeleteConfirmation/BulkDeleteConfirmation.js

+  archiveFiles: PropTypes.bool,
+};
+
+BulkDeleteConfirmation.defaultProps = {


I'm adding default props for now for existing jest tests to pass, will be removed in #1379 and .isRequred will be added to the propType

emteknetnz · 2023-08-08T01:55:23Z

client/src/containers/BulkDeleteConfirmation/DeletionModal.js

+  archiveFiles: PropTypes.bool,
+};
+
+DeletionModal.defaultProps = {


I'm adding default props for now for existing jest tests to pass, will be removed in #1379 and .isRequred will be added to the propType

GuySartorelli

A few things aren't quite right in my manual testing. Note that I checked out #1379 for this manual testing so it should have all the correct strings etc.

The icon for the "archive" option is still the bin:

This doesn't match the icon in userhelp.
The icon at the bottom of the admin for archiving images/files when you select multiple (using the check boxes) is still the bin

GuySartorelli

Looks good, works well locally.

emteknetnz mentioned this pull request Aug 8, 2023

"Files" tab in archive admin is missing silverstripe/silverstripe-versioned-admin#300

Closed

emteknetnz force-pushed the pulls/1.13/archive-text branch 2 times, most recently from cc65e72 to 17924b8 Compare August 8, 2023 00:47

emteknetnz commented Aug 8, 2023

View reviewed changes

emteknetnz force-pushed the pulls/1.13/archive-text branch from 17924b8 to 5374674 Compare August 8, 2023 01:25

emteknetnz marked this pull request as ready for review August 8, 2023 01:25

emteknetnz force-pushed the pulls/1.13/archive-text branch from 5374674 to 6d6cc96 Compare August 8, 2023 01:33

emteknetnz commented Aug 8, 2023

View reviewed changes

emteknetnz force-pushed the pulls/1.13/archive-text branch from 6d6cc96 to 9e5df84 Compare August 8, 2023 01:58

GuySartorelli requested changes Aug 8, 2023

View reviewed changes

ENH Use archive text when file archiving is enabled

fa0fef2

emteknetnz force-pushed the pulls/1.13/archive-text branch from 9e5df84 to fa0fef2 Compare August 8, 2023 03:25

GuySartorelli approved these changes Aug 8, 2023

View reviewed changes

GuySartorelli merged commit 3e5afba into silverstripe:1.13 Aug 8, 2023
12 checks passed

GuySartorelli deleted the pulls/1.13/archive-text branch August 8, 2023 04:08

GuySartorelli mentioned this pull request Aug 20, 2024

API Remove GraphQL #1477

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH Use archive text when file archiving is enabled #1378

ENH Use archive text when file archiving is enabled #1378

emteknetnz commented Aug 8, 2023 •

edited

Loading

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

emteknetnz Aug 8, 2023

GuySartorelli left a comment

GuySartorelli left a comment

ENH Use archive text when file archiving is enabled #1378

ENH Use archive text when file archiving is enabled #1378

Conversation

emteknetnz commented Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuySartorelli left a comment

Choose a reason for hiding this comment

GuySartorelli left a comment

Choose a reason for hiding this comment

emteknetnz commented Aug 8, 2023 •

edited

Loading