Various bug fixes #53
Conversation
|
build now |
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
✏️ could just use the get(id) method and if it throws the NotFoundException you know the config doesn't exist. Also, it seems a little misleading to return true when we get a ReplicationPersistenceException.
There was a problem hiding this comment.
I would agree that we shouldn't return true on a ReplicationPersistenceException
There was a problem hiding this comment.
I changed to return false for now, but I think we will want to throw some sort of error and handle on the front end if we cannot access the persistence store. I can make it do this in the other PR these changes were pulled from.
There was a problem hiding this comment.
I think that is exactly right. We want to throw an exception up if there is an error instead of returning true/false. I am good with this for now as long as we make that happen in the next pr.
| if (!replicationUtils.configExists(id.getValue())) { | ||
| addErrorMessage(ReplicationMessages.configDoesNotExist()); | ||
| } | ||
| } |
There was a problem hiding this comment.
replicationUtils.deleteConfig(id) could be changed to just let the NotFoundException bubble up. Then in the performFunction() method you could just return null with the error message if you catch that exception. At least I think you can do that with the graphql endpoint.
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
I would agree that we shouldn't return true on a ReplicationPersistenceException
|
Successful hero with the test steps above. |
|
build now |
What does this PR do?
Changes pulled from #52
Last Runon history was set, meaning all the metacards would be queried again.Who is reviewing it (please choose AT LEAST two reviewers that need to approve the PR before it can get merged)?
@clockard @kcover @paouelle
How should this be tested? (List steps with links to updated documentation)
Run
log:set DEBUG org.codice.ditto.replication.api.implreplication:delete -c -h -mdeletes replication items in solr as well as the config and all metacards/data.Any background context you want to provide?
What are the relevant tickets?
Screenshots (if appropriate)
Checklist: