Changes related to "Modify Create Program endpoint to require Data Center id" #430
Conversation
…ataCenterId in the request
src/main/java/org/icgc/argo/program_service/exception/ExceptionHandlers.java
Outdated
Show resolved
Hide resolved
src/main/java/org/icgc/argo/program_service/services/ProgramService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/icgc/argo/program_service/services/ProgramService.java
Outdated
Show resolved
Hide resolved
Azher2Ali
changed the title
There was a problem hiding this comment.
Somehow we introduced a changed copyright notice which we don't want. I think it was copied from the mvn library by mistake. In any case, I need the licenses to be copied from the OICR copyright notice text in the following files:
- BadRequestException.java
- ForbiddenException.java
- UnauthorizedException.java
- JWTAuthorizationFilter.java
| /* | ||
| * Copyright (c) 2023. The Ontario Institute for Cancer Research. All rights reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * | ||
| */ |
There was a problem hiding this comment.
This copyright declaration text doesn't match the text from the rest of the files in the repo. Please make sure you are not modifying the content of the copyright so we can keep it consistent. This is the expected text:
/*
* Copyright (c) 2023 The Ontario Institute for Cancer Research. All rights reserved
*
* This program and the accompanying materials are made available under the terms of the GNU Affero General Public License v3.0.
* You should have received a copy of the GNU Affero General Public License along with
* this program. If not, see <http://www.gnu.org/licenses/>.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
* SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
* TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
* OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
* IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
*
*/
| val program = request.getProgram(); | ||
| val admins = request.getAdminsList(); | ||
|
|
||
| // TODO: Refactor this, having a transactional side effect is no longer needed thanks to the |
There was a problem hiding this comment.
What is involved in refactoring this? I would prefer to build this right while we are looking at it instead of hoping someone addresses this in the future.
|
After looking at the Jenkins build results, it seems some additional work is required. |
| if (dataCenterId != null) { | ||
| val dataCenterEntity = dataCenterRepository.findById(dataCenterId); | ||
| if (!dataCenterEntity.isEmpty()) { | ||
| programEntity.setDataCenterId(dataCenterId); | ||
| } else { | ||
| throw new RecordNotFoundException("DataCenterId '" + dataCenterId + "' not found"); | ||
| } | ||
| } else { | ||
| throw new BadRequestException("DataCenterId cannot be null or empty"); | ||
| } |
There was a problem hiding this comment.
Think I'd invert these negative conditions, to facilitate reading:
e.g. check if datacenterID == null throw... otherwise (no need for an else), check if dataCenterEntity is empty, and then throw... otherwise (without an else) set the Id for the programEntity
the negative conditions, apart from adding unnecessary/avoidable cognitive burden, generate this nested code that complicates things.
There was a problem hiding this comment.
The proposed code adjustments prioritize clarity and simplicity by reversing negative conditions and streamlining the structure. Through this modification, the code achieves improved readability and a more straightforward flow and the changes have been added accordingly.
…ges related to test case failures
| if (dataCenterId == null) { | ||
| throw new BadRequestException("DataCenterId cannot be null or empty"); | ||
| } | ||
|
|
||
| val dataCenterEntity = dataCenterRepository.findById(dataCenterId); | ||
| if (dataCenterEntity.isEmpty()) { | ||
| throw new RecordNotFoundException("DataCenterId '" + dataCenterId + "' not found"); | ||
| } | ||
| programEntity.setDataCenterId(dataCenterId); |
There was a problem hiding this comment.
Nice! thanks for making this tiny change.
src/main/java/org/icgc/argo/program_service/services/ProgramService.java
Outdated
Show resolved
Hide resolved
…rvice.java Co-authored-by: Anders Richardsson <[email protected]>
Enhanced Create Program Endpoint: Added Data Center ID Parameter and Improved Error Handling
In response to the requirements outlined in issue #413, this commit introduces several important enhancements to the Create Program endpoint:
Data Center ID Parameter: Programs can now be created with a designated Data Center ID. This change allows for improved data management and resource allocation, as programs can be associated with specific data centers.
Error Handling for Invalid Data Center IDs: The endpoint now returns an HTTP 400 error when an invalid Data Center ID is provided. The error response includes a descriptive message that explains why the request failed, helping users understand and address the issue.
The screenshot elaborates that we have added the data centre id in the request payload of create program api and validating it's presence against the data center table, the data center id which is present in the payload does not exist in the data centre table and thus it returns an error as Bad Request with HTTP Status 400 and displays a message as "Data CenterId not found"