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 #418: Change the parameter name on the URL #429

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

RomanMagnoli
Copy link
Contributor

EPL-1535: The URL parameter name was changed in order to match with the parameter name itself.

EPL-1535: The URL parameter name was changed in order to match with the parameter name itself.
@RomanMagnoli RomanMagnoli self-assigned this Jun 25, 2024
@RomanMagnoli RomanMagnoli linked an issue Jun 25, 2024 that may be closed by this pull request
@RomanMagnoli RomanMagnoli added the bug Something isn't working label Jun 25, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

GPT Review for doc.yaml

Review

  • Estimated effort to review [1-5]: 1, because the changes are minimal and straightforward, involving only the renaming of path parameters in a YAML file.
  • Score: 95

Code feedback

  • File:
    modules_core/com.smf.securewebservices/web/com.smf.securewebservices/doc/doc.yaml
  • Language:
    yaml
  • Suggestion:
    Ensure that the new parameter name _entityName is consistently used throughout the codebase and documentation. This includes updating any references in the backend code, frontend code, and any other related documentation. [important]
  • Label:
    best practice
  • Existing code:
  /sws/com.smf.securewebservices.obRest/{modelName}:
  /sws/com.smf.securewebservices.obRest/{modelName}/{recordID}:
  • Improved code:
  /sws/com.smf.securewebservices.obRest/{_entityName}:
  /sws/com.smf.securewebservices.obRest/{_entityName}/{recordID}:

This comment has been minimized.

EPL-1535: Add a 'lowerCase' for the parameter in order to find the desired property.
Copy link
Collaborator

Choose a reason for hiding this comment

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

GPT Review for SecureJsonDataService.java

Review

  • Estimated effort to review [1-5]: 2, because the changes are minor and involve simple refactoring using utility methods. The logic and structure of the code remain largely unchanged.
  • Score: 95

Code feedback

  • File:
    modules_core/com.smf.securewebservices/src/com/smf/securewebservices/service/SecureJsonDataService.java
  • Language:
    java
  • Suggestion:
    Using StringUtils.equals and StringUtils.lowerCase from Apache Commons Lang is a good practice for null-safe operations. However, ensure that the parameters.get method does not return null, or handle the null case explicitly to avoid potential NullPointerException. [important]
  • Label:
    best practice
  • Existing code:
if (StringUtils.equals("true", parameters.get(JsonConstants.SHOW_FK_DROPDOWN_UNFILTERED_PARAMETER))) {
  • Improved code:
if (StringUtils.equals("true", StringUtils.defaultString(parameters.get(JsonConstants.SHOW_FK_DROPDOWN_UNFILTERED_PARAMETER)))) {
  • File:
    modules_core/com.smf.securewebservices/src/com/smf/securewebservices/service/SecureJsonDataService.java
  • Language:
    java
  • Suggestion:
    Ensure that the parameters.get method does not return null, or handle the null case explicitly to avoid potential NullPointerException when using StringUtils.lowerCase. [important]
  • Label:
    best practice
  • Existing code:
final String distinctPropertyPath = StringUtils.lowerCase(parameters.get(JsonConstants.DISTINCT_PARAMETER));
  • Improved code:
final String distinctPropertyPath = StringUtils.lowerCase(StringUtils.defaultString(parameters.get(JsonConstants.DISTINCT_PARAMETER)));

Copy link
Collaborator

Choose a reason for hiding this comment

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

GPT Review for doc.yaml

Review

  • Estimated effort to review [1-5]: 1, because the changes are straightforward and involve minor modifications to the endpoint paths and descriptions in the YAML file.
  • Score: 95

Code feedback

  • File:
    modules_core/com.smf.securewebservices/web/com.smf.securewebservices/doc/doc.yaml
  • Language:
    yaml
  • Suggestion:
    The changes made to the endpoint paths and descriptions are clear and improve the clarity of the API documentation. However, ensure that the changes are consistently applied across all related documentation and code to avoid discrepancies. [important]
  • Label:
    best practice
  • Existing code:
  /sws/com.smf.securewebservices.obRest/{modelName}:
  /sws/com.smf.securewebservices.obRest/{modelName}/{recordID}:
  • Improved code:
  /sws/com.smf.securewebservices.obRest/{_entityName}:
  /sws/com.smf.securewebservices.obRest/{_entityName}/{recordID}:

Copy link

Failed

  • E Maintainability Rating on New Code (is worse than A)

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 1 Code Smell

Coverage and Duplications

  • Coverage 0.00% Coverage (0.00% Estimated after merge)
  • Duplications 0.00% Duplicated Code (13.20% Estimated after merge)

Project ID: etendosoftware_etendo_core_AYOKvQCAuJ79WHyLB97g

View in SonarQube

@valeg-etendo valeg-etendo merged commit a5b467d into main Jun 26, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EPL-1535 : Swagger API error - Bad filtering by entity
6 participants