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

Issue#dcc2983 - Fix for preventing adding a Contributor via Contributor #3062

Conversation

johnpinto1
Copy link
Contributor

@johnpinto1 johnpinto1 commented Nov 4, 2021

page if both Name and Email are not present.

Fixes DCC bug #2983
spec/services/api/v1/contextual_error_service_spec.rb
Changes:

  • Changes in Contributor model:
    • Renamed validation method name_or_email_presence() -> name_and_email_presence()
    • Updated conditions in name_and_email_presence() method to return errors if Name or Email missing.
  • Updated tests in spec/models/contributor_spec.rb and spec/services/api/v1/contextual_error_service_spec.r to reflect change in Contributor model.

@briri Not sure if this conditionality is different for UC3.
(Away for a week so won't pick up until Nov 15th.)

Scrrenshots of different error scenarios:
Selection_031
Selection_030
Selection_029

page if both Name and Email are not present.

Fixes DCC bug #2983

Changes:
  - Changes in Contributor model:
       - Renamed validation method name_or_email_presence() -> name_and_email_presence()
       - Updated conditions in name_and_email_presence() method to return errors if Name or Email missing.
  - Updated tests in spec/models/contributor_spec.rb and spec/services/api/v1/contextual_error_service_spec.rb to reflect change in
Contributor model.
@johnpinto1 johnpinto1 force-pushed the bug_DCC2983-possible_to_add_contributor_without_a_name branch from 2a3f695 to 26b4976 Compare November 4, 2021 12:43
@johnpinto1
Copy link
Contributor Author

Ignore my previous comment on tests. I deleted comment and fixed broken test.

Copy link
Contributor

@raycarrick-ed raycarrick-ed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me.

@briri
Copy link
Contributor

briri commented Nov 17, 2021

Yes, this will be problematic for us as we have many records where email was not provided.

We only have a handful of records with a nil name, so I can manually address that one fairly easily. Maybe set this as an additional config flag like: x.application.require_contributor_emails = false

@johnpinto1
Copy link
Contributor Author

@raycarrick-ed @briri Will close issue and re-open after finding a way of reconciling with @briri comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants