Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Provide better feedback in the case where a capture cannot be taken (for example, if there was an error retrieving content) by displaying an error in the 'message' field in the dashboard.
For reference, the analogous change in the Wordpress version of the plugin is: https://plugins.trac.wordpress.org/changeset?sfp_email=&sfph_mail=&repon...
Comment | File | Size | Author |
---|---|---|---|
#8 | amber-display_message_dashboard_capture_error-2717693-8.patch | 7.4 KB | skbly7 |
Comments
Comment #2
skbly7 CreditAttribution: skbly7 as a volunteer commentedSteps followed:
- Update of "libraries" directory with latest "amber_common" sync.
- The size column has been loaded with responsibility to display errors instead of defining new column:
- Reason: Already too many columns, thus message column was felt unnecessary.
Feature Addition/Modification:
- Removal of Up/Down value in case of "size too large" errors to avoid confusion.
- Addition of error message in case of "size too large" in size column.
Comment #3
skbly7 CreditAttribution: skbly7 as a volunteer commentedSteps followed:
- Update of "libraries" directory with latest "amber_common" sync.
- The size column has been loaded with responsibility to display errors instead of defining new column:
- Reason: Already too many columns, thus message column was felt unnecessary.
Feature Addition/Modification:
- Removal of Up/Down value in case of "size too large" errors to avoid confusion.
- Addition of error message in case of "size too large" in size column.
Feature related file changed are: "amber.pages.inc" and "amber.batch.inc".
Patch for both 7.x and 7.0 branch is attached.
Comment #6
skbly7 CreditAttribution: skbly7 as a volunteer commentedPatch included:
- libraries folder latest synced.
- feature implementation
- display of message in "size" column (reason: there were already too many columns, so instead of adding new "message" column, size was overloaded with message's functionality.
- removal of up/down message in case of "size too large" error.
File changes description:
- All "libraries" folder change are because of sync.
- The other two files to add functionalities.
Comment #7
jlicht CreditAttribution: jlicht commentedRegarding
Two comments:
* I don't think we need to provide a default value for $last_checked.
* The logic for checking $last_checked behaves differently if the value is NULL and if it's an empty string - this is confusing.
I think this function can be similar to the Wordpress one:
If you agree, please preroll the patch and resubmit.
Comment #8
skbly7 CreditAttribution: skbly7 as a volunteer commentedIt has been mistype from my side on "_amber_status_column_value" function, I intended to have NULL as default value instead of "2 ", but ended up submitting patch of wrong function. I am extremely sorry for it, and will take care for future patches.
Also, I agree on both your comments and have removed default value from the last_checked parameter. Resubmitting the patch with updated function.
Comment #9
jlicht CreditAttribution: jlicht commentedThe change to the _amber_status_column_value() function looks good.
One more change before applying this patch - PHP close tags have been added to a few of the files. This is not desired - see https://www.drupal.org/coding-standards#phptags
Comment #10
jlicht CreditAttribution: jlicht commentedComment #12
jlicht CreditAttribution: jlicht commentedCommitted a change that builds upon the previous patches, but adds some more features:
* Adds an extra column ('message') to the amber_cache table which allows us to track errors associated with the caching process for a specific backend, rather than for the checking process, which is backend-agnostic. For example, if you are caching to Perma and your local disk, you might receive an error from Perma, but still be able to save locally. This column allows us to keep the error message associated with the backend, and display appropriately on the frontend
* Updated the AmberFetcher, PermaFetcher, and InternetArchiveFetcher to provide more information in case of errors (e.g. url, provider id) rather than just an error message string - which allows us to save an error message and associate it with a specific cache attempt with a specific backend