Add org_id to api_clients table and refactor of API v1 services #2725
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ier to manage and to provide better errors to clients
… from local machine time
xsrust
approved these changes
Nov 23, 2020
Comment on lines
+17
to
+33
| Plan.transaction do | ||
| plan.funder = safe_save_org(org: plan.funder) | ||
| plan.grant = safe_save_identifier(identifier: plan.grant) | ||
|
|
||
| plan.save | ||
|
|
||
| plan.identifiers.each do |id| | ||
| id.identifiable = plan.reload | ||
| safe_save_identifier(identifier: id) | ||
| end | ||
| plan.contributors.each do |contributor| | ||
| contributor.plan = plan.reload | ||
| safe_save_contributor(contributor: contributor) | ||
| end | ||
|
|
||
| plan.reload | ||
| end |
There was a problem hiding this comment.
I'm not sure if there's anywhere else in the application we generate multiple records like this (other than the Answers#create_or_update) but it would be good to be more consistent in using this Model.transaction do style save for any of those as well.
Comment on lines
+37
to
+51
| def contextualize(errors:, context: "DMP") | ||
| errs = errors.is_a?(ActiveModel::Errors) ? errors.full_messages : [] | ||
| errs = errors if errors.is_a?(Array) && errs.empty? | ||
| return errs unless errs.any? | ||
|
|
||
| errs.map do |msg| | ||
| msg.gsub("org name", "affiliation name") | ||
| .gsub("identifiers value", "identifier values") | ||
| .gsub("Contributors", "Contact/Contributor") | ||
| .gsub(/^Identifiers/, "#{context.capitalize} identifier") | ||
| .gsub(/^Name/, "#{context.capitalize} name") | ||
| .gsub(/^Title/, "#{context.capitalize} title") | ||
| .gsub(/^Value/, "#{context.capitalize} value") | ||
| end | ||
| end |
There was a problem hiding this comment.
Cool approach to making the ActiveModel errors make sense to the API users without needing to re-invent the wheel:)
portagenetwork
pushed a commit
to portagenetwork/roadmap
that referenced
this pull request
Feb 24, 2022
…oadmap#2725) * updated contextualize_error_service * added org_id to api_clients. Refactored API services to make them easier to manage and to provide better errors to clients * updated test that fails randomly * updated api to use new persistenceservice * fixed rubocop issue * remove rubocop disable that my machine wants butgithub actions does not * remove rubocop disable that my machine wants butgithub actions does not * remove rubocop disable that my machine wants butgithub actions does not * fixed issue with safe_date tests because Travis instance time differs from local machine time * fix for randomly failing test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
plan.savewould result in errors if it had 2 contributors belonging to the same org. The first org would get saved but the subsequent attempt would result in a duplicate record error. The methods in this class essentially just do afind_or_create. The entire process is wrapped inside a transaction so that if any failures are encountered the entire transaction is aborted.{ contact: { email: '[email protected]' }and{ contributor: { email: '[email protected]', name: 'John Doe' }are combined into one single entryI am considering adding a rake task that will take a json input and run it through these services to validate/provide feedback so that we can test the API more easily and also assist with debugging.