Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

community auto-submit: remove notification about request auto-acceptance #203

Open
Labels
bug Something isn't working
Milestone

Comments

@anikachurilova
Copy link
Contributor

anikachurilova commented Oct 7, 2024

If the community's review_policy is set to open or members, then users are able to submit a record to a community, bypassing the review process. In this case, the review request is still created and auto-accepted by System.

The problem: request acceptance notification is sent to the request creator, and as this accept action is made by system user, we are getting an exception. This notification is probably not needed at all, because as user is submitting the record directly, they don't even need to know that there was a request created.

Sentry exception: https://cds-sentry.web.cern.ch/sentry/cds-rdm/issues/34056/events/816855/

@anikachurilova anikachurilova added the bug Something isn't working label Oct 7, 2024
@anikachurilova anikachurilova added this to the SSPN milestone Oct 7, 2024
@anikachurilova anikachurilova changed the title community auto-submit: remove notification about request acceptance for system user community auto-submit: remove notification about request auto-acceptance Nov 8, 2024
anikachurilova added a commit to anikachurilova/invenio-rdm-records that referenced this issue Nov 8, 2024
@anikachurilova anikachurilova moved this from Ready to In review 🔎 in Sprint Q4/2024 🍂 Nov 8, 2024
@anikachurilova anikachurilova removed their assignment Nov 8, 2024
zzacharo pushed a commit to inveniosoftware/invenio-rdm-records that referenced this issue Nov 8, 2024
@github-project-automation github-project-automation bot moved this from In review 🔎 to To release 🤖 in Sprint Q4/2024 🍂 Nov 8, 2024
@ntarocco ntarocco reopened this Nov 8, 2024
@ntarocco
Copy link
Contributor

ntarocco commented Nov 8, 2024

We discussed quickly in IRL, can we solve this differently? We might want to have in the future that the system will send automatic e-mails, it might make sense.

By the way it looks like that we cannot access the exception?

So I would split the fix in 2 parts:

  1. make system identity that can send notifications: this might mean fixing some notifications/builders logic (depending on the issue)

  2. exclude sending notifications on auto-accept. For this part, we could do the following:

    1. inject a param in the auto-accept execute_action
    2. allow kwargs here
    3. implement the condition here based on this param

    Another way would be to create a new action, very similar to Accept (e.g. AutoAccept), and avoid the notification. However, I am not sure how hard it is that, and if it is actually better...

@ntarocco ntarocco reopened this Nov 11, 2024
@anikachurilova anikachurilova moved this from To release 🤖 to In progress in Sprint Q4/2024 🍂 Nov 12, 2024
anikachurilova added a commit to anikachurilova/invenio-rdm-records that referenced this issue Nov 14, 2024
anikachurilova added a commit to anikachurilova/invenio-requests that referenced this issue Nov 14, 2024
@anikachurilova anikachurilova removed their assignment Nov 14, 2024
anikachurilova added a commit to anikachurilova/invenio-requests that referenced this issue Nov 14, 2024
anikachurilova added a commit to anikachurilova/invenio-rdm-records that referenced this issue Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment