captcha on request copy #9235
captcha on request copy #9235
Conversation
| @@ -1579,6 +1579,12 @@ solr-database-resync.cron = 0 15 2 * * ? | |||
|
|
|||
| registration.verification.enabled = false | |||
|
|
|||
| #---------------------------------------------------------------# | |||
There was a problem hiding this comment.
could you consider to add a new modules/google-captcha.cfg file? I think there are several configurations for Google Captcha. Perhaps it already deserves a single configuration file for it.
And also add a reference for it on dspace.cfg:
include = ${module_dir}/google-captcha.cfg
There was a problem hiding this comment.
I think this configuration can remain in dspace.cfg for now, unless you want to move everything related to Google Captcha to the same google-captcha.cfg file (as @paulo-graca mentions above). So, I'd say that new google-captcha.cfg is optional. It's nice to have, but it can be done by in a different PR if needed.
However, I'd ask that you please group these related configurations together by removing the new "GOOGLE CAPTCHA CONFIGURATION" header on lines 1582-1584. That's not necessary to add again because that same header already exists on lines 1575-1577 in this same file.
There was a problem hiding this comment.
@GauravD2t : I reviewed/tested this today alongside the frontend PR. See more details in my review of that PR
I have some additional suggestions in this PR... see inline below.
| } catch (InvalidReCaptchaException e) { | ||
| throw new InvalidReCaptchaException(e.getMessage(), e); | ||
| } | ||
| } |
There was a problem hiding this comment.
As noted by Codecov (in the "Files Changed" tab on this PR), this new code doesn't have any tests written for it.
We should add tests for Captchas to the RequestItemRepositoryIT. These tests could be similar to / modeled on the Captcha tests that also exist in RegistrationRestRepositoryIT (search for "captcha" in this file): https://github.com/DSpace/DSpace/blob/main/dspace-server-webapp/src/test/java/org/dspace/app/rest/RegistrationRestRepositoryIT.java
| @@ -1579,6 +1579,12 @@ solr-database-resync.cron = 0 15 2 * * ? | |||
|
|
|||
| registration.verification.enabled = false | |||
|
|
|||
| #---------------------------------------------------------------# | |||
There was a problem hiding this comment.
I think this configuration can remain in dspace.cfg for now, unless you want to move everything related to Google Captcha to the same google-captcha.cfg file (as @paulo-graca mentions above). So, I'd say that new google-captcha.cfg is optional. It's nice to have, but it can be done by in a different PR if needed.
However, I'd ask that you please group these related configurations together by removing the new "GOOGLE CAPTCHA CONFIGURATION" header on lines 1582-1584. That's not necessary to add again because that same header already exists on lines 1575-1577 in this same file.
|
@GauravD2t : Unfortunately this feature missed our feature merger deadlines for 8.0 (as feedback was not addressed in time). It will need to be rescheduled for 9.0. |
References
Add references/links to any related issues or PRs. These may include:
Description
requestCopy.verification.enabled=true/false in the configuration file
Instructions for Reviewers
Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.
List of changes in this PR:
Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.