-
Notifications
You must be signed in to change notification settings - Fork 9
Works v2 specs #15
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
base: master
Are you sure you want to change the base?
Works v2 specs #15
Conversation
Update measurement service schema
…revision (#19) * PFM-4800 - Estimate service and Contract service API Spec update for revision * Updated contractLineItemRef * Updated estimate contract
Statement and rate analysis api specs
WalkthroughThe recent changes across various YAML files enhance the API specifications for services such as contracts, estimates, measurements, rate analysis, and statements. New properties and endpoints have been introduced, along with improved formatting for clarity. These modifications collectively enhance the structure and functionality of the APIs, supporting better data management and integration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Database
User->>API: Request to create contract
API->>Database: Save contract details
Database-->>API: Confirmation
API-->>User: Success response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 51
Outside diff range, codebase verification and nitpick comments (9)
Domain Services/Works/Statement-v1.0.0.yaml (4)
12-12
: Improve the endpoint description.The description "Create Statement" is too generic. Consider providing a more detailed description.
- description: Create Statement + description: Create a new analysis statement in the system.
36-36
: Improve the endpoint description.The description "Search Statement" is too generic. Consider providing a more detailed description.
- description: Search Statement + description: Search for an existing analysis statement in the system.
60-60
: Improve the endpoint description.The description "Create Statement" is too generic. Consider providing a more detailed description.
- description: Create Statement + description: Create a new utilization statement in the system.
84-84
: Improve the endpoint description.The description "Search Statement" is too generic. Consider providing a more detailed description.
- description: Search Statement + description: Search for an existing utilization statement in the system.Domain Services/Works/Contract-Service-v1.0.0.yaml (3)
Line range hint
17-17
: Fix typo in response description.There is a typo in the response description. "Accepted create contract request." should be "Accepted create contract request".
- description: Accepted create contract request. + description: Accepted create contract request.Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
Line range hint
27-27
: Fix typo in response description.There is a typo in the response description. "Accepted create contract request." should be "Accepted update contract request".
- description: Accepted create contract request. + description: Accepted update contract request.Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
Line range hint
44-44
: Fix typo in response description.There is a typo in the response description. "Successful response sorted by reverse chrnological order of creation" should be "Successful response sorted by reverse chronological order of creation".
- description: Successful response sorted by reverse chrnological order of creation + description: Successful response sorted by reverse chronological order of creationTools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
Domain Services/Works/Estimate-service-v1.0.0.yaml (2)
36-37
: Improve the endpoint description.The description "Submit updated Estimate with the estimate details to update an existing estimate in the system" is too verbose. Consider providing a more concise description.
- description: >- - Submit updated Estimate with the estimate details to update an existing - estimate in the system + description: Update an existing estimate with the provided details.
63-64
: Improve the endpoint description.The description "Gets the list of estimates for a particular tenant based on search criteria" is too generic. Consider providing a more detailed description.
- description: > - Gets the list of estimates for a particular tenant based on search - criteria. + description: Retrieve a list of estimates for a specific tenant based on the provided search criteria.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- Domain Services/Works/Contract-Service-v1.0.0.yaml (2 hunks)
- Domain Services/Works/Estimate-service-v1.0.0.yaml (6 hunks)
- Domain Services/Works/Measurement-Book-v1.0.0.yaml (1 hunks)
- Domain Services/Works/Rate-Analysis-v1.0.0.yaml (1 hunks)
- Domain Services/Works/Statement-v1.0.0.yaml (1 hunks)
- Domain Services/Works/sor-rates-1.0.0.yaml (1 hunks)
Additional context used
yamllint
Domain Services/Works/Statement-v1.0.0.yaml
[warning] 59-59: too many spaces after colon
(colons)
[warning] 230-230: too many blank lines
(3 > 2) (empty-lines)
[error] 282-282: no new line character at the end of file
(new-line-at-end-of-file)
Domain Services/Works/Estimate-service-v1.0.0.yaml
[error] 187-187: trailing spaces
(trailing-spaces)
[error] 271-271: trailing spaces
(trailing-spaces)
[error] 324-324: trailing spaces
(trailing-spaces)
[error] 328-328: trailing spaces
(trailing-spaces)
[error] 349-349: trailing spaces
(trailing-spaces)
[error] 357-357: trailing spaces
(trailing-spaces)
[error] 364-364: trailing spaces
(trailing-spaces)
[error] 391-391: trailing spaces
(trailing-spaces)
[error] 456-456: no new line character at the end of file
(new-line-at-end-of-file)
Domain Services/Works/Measurement-Book-v1.0.0.yaml
[error] 6-6: trailing spaces
(trailing-spaces)
[error] 73-73: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 151-151: trailing spaces
(trailing-spaces)
[error] 177-177: trailing spaces
(trailing-spaces)
[error] 246-246: trailing spaces
(trailing-spaces)
[error] 268-268: trailing spaces
(trailing-spaces)
[error] 274-274: trailing spaces
(trailing-spaces)
[error] 279-279: trailing spaces
(trailing-spaces)
[error] 293-293: trailing spaces
(trailing-spaces)
[error] 324-324: trailing spaces
(trailing-spaces)
[warning] 335-335: wrong indentation: expected 10 but found 12
(indentation)
[error] 336-336: trailing spaces
(trailing-spaces)
[error] 371-371: trailing spaces
(trailing-spaces)
[error] 383-383: trailing spaces
(trailing-spaces)
[error] 385-385: trailing spaces
(trailing-spaces)
[error] 396-396: trailing spaces
(trailing-spaces)
Domain Services/Works/Rate-Analysis-v1.0.0.yaml
[warning] 11-11: too many spaces after hyphen
(hyphens)
[warning] 113-113: too many blank lines
(4 > 2) (empty-lines)
[warning] 250-250: too many spaces after colon
(colons)
[warning] 421-421: too many blank lines
(8 > 2) (empty-lines)
[warning] 434-434: too many blank lines
(3 > 2) (empty-lines)
[error] 537-537: no new line character at the end of file
(new-line-at-end-of-file)
Domain Services/Works/sor-rates-1.0.0.yaml
[error] 93-93: trailing spaces
(trailing-spaces)
[error] 175-175: trailing spaces
(trailing-spaces)
[warning] 272-272: too many spaces after colon
(colons)
[error] 291-291: trailing spaces
(trailing-spaces)
[error] 294-294: trailing spaces
(trailing-spaces)
[error] 308-308: trailing spaces
(trailing-spaces)
[error] 342-342: trailing spaces
(trailing-spaces)
[error] 357-357: trailing spaces
(trailing-spaces)
[error] 358-358: trailing spaces
(trailing-spaces)
[error] 386-386: trailing spaces
(trailing-spaces)
[error] 435-435: trailing spaces
(trailing-spaces)
[error] 456-456: trailing spaces
(trailing-spaces)
[error] 457-457: trailing spaces
(trailing-spaces)
[error] 472-472: trailing spaces
(trailing-spaces)
[error] 493-493: trailing spaces
(trailing-spaces)
[error] 498-498: no new line character at the end of file
(new-line-at-end-of-file)
checkov
Domain Services/Works/Statement-v1.0.0.yaml
[HIGH] 1-283: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-283: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
Domain Services/Works/Measurement-Book-v1.0.0.yaml
[HIGH] 1-410: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-410: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
Domain Services/Works/Rate-Analysis-v1.0.0.yaml
[HIGH] 1-538: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-538: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
Domain Services/Works/sor-rates-1.0.0.yaml
[HIGH] 1-499: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[HIGH] 1-499: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
Additional comments not posted (11)
Domain Services/Works/Statement-v1.0.0.yaml (4)
2-5
: LGTM!The
info
section is correctly formatted and contains the necessary metadata.
105-111
: LGTM!The
StatementCreateRequest
schema is correctly formatted and references the necessary components.
113-127
: LGTM!The
StatementRequest
schema is correctly formatted and contains the necessary properties.
138-169
: LGTM!The
Statement
schema is correctly formatted and contains the necessary properties.Domain Services/Works/Contract-Service-v1.0.0.yaml (4)
Line range hint
2-4
: LGTM!The
info
section is correctly formatted and contains the necessary metadata.Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
Line range hint
272-277
: LGTM!The
ContractCriteria
schema is correctly formatted and contains the necessary properties.Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
Line range hint
398-422
: LGTM!The
ContractRequest
schema is correctly formatted and contains the necessary properties.Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
103-108
: Fix formatting issue.There is a formatting issue with the example value. The example values should be separated by commas.
- example: CONTRACT, REVISION-CONTRACT + example: CONTRACT, REVISION-CONTRACTLikely invalid or redundant comment.
Domain Services/Works/Estimate-service-v1.0.0.yaml (1)
Line range hint
2-4
: LGTM!The
info
section is correctly formatted and contains the necessary metadata.Domain Services/Works/sor-rates-1.0.0.yaml (2)
472-472
: Remove trailing spaces.There are trailing spaces at the end of this line
Tools
yamllint
[error] 472-472: trailing spaces
(trailing-spaces)
6-6
: Remove trailing spaces.There are trailing spaces at the end of this line.
- +Likely invalid or redundant comment.
schema: | ||
$ref: >- | ||
https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/ErrorRes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 73-73: trailing spaces
(trailing-spaces)
$ref: '#/components/schemas/MeasurementCriteria' | ||
pagination: | ||
$ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/Pagination' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 396-396: trailing spaces
(trailing-spaces)
currentValue: | ||
type: number | ||
description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements. | ||
cumulativeValue: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
cumulativeValue: | |
cumulativeValue: |
$ref: >- | ||
https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/ErrorRes | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 100-100: trailing spaces
(trailing-spaces)
default: 1 | ||
currentValue: | ||
type: number | ||
description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
- description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements.
+ description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements. | |
description: Derived from LBHQ as L*B*H*Q or can be entered directly without LBHQ measurements. |
Tools
yamllint
[error] 293-293: trailing spaces
(trailing-spaces)
type: object | ||
description: 'Any additional information that needs to be attached to the sor. For attachments, fileStoreIds will also be stored here. ' | ||
required: [ amount ] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 456-456: trailing spaces
(trailing-spaces)
type: string | ||
description: 'SOR code or head code' | ||
example: "MR" | ||
maxLength: 128 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove excessive spaces after colon.
There are too many spaces after the colon in this section.
- description: Sors or ExtraCharges
+ description: Sors or ExtraCharges
Committable suggestion was skipped due to low confidence.
Tools
yamllint
[warning] 272-272: too many spaces after colon
(colons)
required: | ||
- ResponseInfo | ||
- sors | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 357-357: trailing spaces
(trailing-spaces)
- required: | ||
- code | ||
- description | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
yamllint
[error] 294-294: trailing spaces
(trailing-spaces)
required: | ||
- sorId | ||
oneOf: | ||
- required: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing spaces.
There are trailing spaces at the end of this line.
-
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- required: | |
- required: |
Tools
yamllint
[error] 291-291: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (10)
Domain Services/Works/Muster-Roll-Service-v1.0.0.yaml (1)
69-73
: Add a description for thetenantId
parameterThe addition of the
tenantId
parameter is good for multi-tenant support. However, it would be helpful to add a description explaining its purpose and any constraints (e.g., required/optional).Consider adding a description like this:
- name: tenantId in: query description: Unique identifier of the tenant for which the muster roll is being searched required: true # if it's a mandatory parameter schema: type: string example: pb.amritsarDomain Services/Works/Estimate-service-v1.0.0.yaml (5)
166-166
: Enhanced Estimate schema with improved property definitionsThe updates to the
Estimate
schema, including new properties, improved descriptions, and added constraints, significantly enhance the clarity and structure of the API. The additions ofrevisionNumber
andbusinessService
properties, along with examples and pattern constraints, provide better guidance for API consumers.However, some descriptions could be further improved for clarity:
- Line 199: Consider specifying why the Project ID is not updatable.
- Line 218: Add examples of possible workflow states for better understanding.
- Line 250: Fix the typo "fro mdms" to "from mdms".
Consider updating these descriptions for improved clarity.
Also applies to: 168-168, 174-174, 180-181, 185-196, 199-199, 204-206, 218-218, 221-221, 224-224, 227-228, 231-233, 236-237, 240-243, 246-247, 250-250, 258-258, 269-271
283-283
: Improved EstimateDetail schema with additional measurement propertiesThe updates to the
EstimateDetail
schema, including new properties for dimensions (length
,width
,height
),quantity
, andisDeduction
, significantly enhance the flexibility and precision of estimates. The addition ofprevLineitemId
supports estimate revisions, which is a valuable feature.Suggestion for improvement:
For theisDeduction
property (line 335), consider adding an example to clarify its usage:isDeduction: type: boolean description: Whether this is a deduction or an addition + example: false
This will help API consumers understand how to use this property correctly.
Also applies to: 285-285, 289-289, 291-291, 296-297, 301-302, 305-305, 310-310, 313-313, 316-316, 320-320, 322-336, 343-343, 347-351
370-370
: Improved AmountDetail schema with clearer property descriptionsThe updates to the
AmountDetail
schema, particularly the addition of descriptions for various properties, enhance the clarity and usability of the API. The description for theamount
property is especially helpful in understanding its role within the estimate.Suggestion for improvement:
For thetype
property (line 376), consider providing a more comprehensive list of examples:type: type: string description: Amount type master - example: 'Gst, cess, charge ' + example: 'GST, CESS, CHARGE, LABOR_CHARGE, MATERIAL_COST'This will give API consumers a better understanding of the possible values for this field.
Also applies to: 372-372, 376-376, 380-380, 383-383, 389-391
408-408
: Improved workflow property descriptions in EstimateRequestThe updates to the descriptions of the
workflow
object properties inEstimateRequest
enhance the clarity of the API documentation. The explanations provide valuable context for API consumers, particularly for theassignees
property.Suggestion for improvement:
In theassignees
description (lines 416-418), there's a minor typo that should be corrected:description: >- UUID of the users in the system to assign workflow to the - specific user intead of a all the users with the gien role. + specific user instead of all the users with the given role.This correction will improve the readability and professionalism of the documentation.
Also applies to: 411-411, 416-418
187-187
: Address minor YAML formatting issuesThe static analysis tool has identified some minor formatting issues in the YAML file:
- Trailing spaces on lines 187, 271, 324, 328, 349, 357, 364, and 391.
- Missing new line character at the end of the file (line 456).
While these don't affect functionality, addressing them can improve readability and version control efficiency.
Please remove the trailing spaces from the mentioned lines and ensure there's a new line character at the end of the file.
Also applies to: 271-271, 324-324, 328-328, 349-349, 357-357, 364-364, 391-391, 456-456
🧰 Tools
🪛 yamllint
[error] 187-187: trailing spaces
(trailing-spaces)
Domain Services/Works/Organisation-V1.0.0.yaml (4)
5-11
: Enhance the service description and approve new tagsThe simplified description in the info section is concise, but it might benefit from additional details about the service's purpose and capabilities. Consider expanding it to provide more context for API consumers.
The new tags "OrgServices" and "Organisation" are a good addition, as they help categorize and organize the API operations effectively.
Would you like assistance in drafting a more comprehensive service description?
Line range hint
14-146
: Approve new paths and error response updatesThe addition of new paths for create, update, and search operations provides a clear and structured API for managing organisations. The separation of "OrgServices" and "Organisation" endpoints allows for flexibility in implementation.
The update to error response schemas, referencing a versioned contract, is a good practice for maintaining API consistency and versioning.
Consider adding a brief description for each path to provide more context about the operation's purpose and any specific requirements.
457-588
: Approve new schemas and suggest minor improvementThe addition of Address, Boundary, Document, and GeoLocation schemas significantly enhances the API's capability to handle complex data structures. These well-defined schemas provide a clear structure for representing various aspects of an organisation and its related data.
The new schemas improve the overall completeness and flexibility of the API.
Consider adding example values for key properties in these new schemas to improve the API documentation and make it easier for consumers to understand the expected data format.
🧰 Tools
🪛 yamllint
[error] 588-588: no new line character at the end of file
(new-line-at-end-of-file)
12-12
: Address minor YAML formatting issuesThere are two minor formatting issues in the YAML file:
- Trailing spaces on line 12
- Missing new line character at the end of the file
To improve adherence to YAML best practices and enhance readability, please make the following changes:
- Remove trailing spaces from line 12
- Add a new line character at the end of the file (after line 588)
These changes will resolve the yamllint warnings and improve the overall quality of the YAML file.
Also applies to: 588-588
🧰 Tools
🪛 yamllint
[error] 12-12: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- Domain Services/Works/Estimate-service-v1.0.0.yaml (5 hunks)
- Domain Services/Works/Muster-Roll-Service-v1.0.0.yaml (5 hunks)
- Domain Services/Works/Organisation-V1.0.0.yaml (13 hunks)
🧰 Additional context used
🪛 yamllint
Domain Services/Works/Estimate-service-v1.0.0.yaml
[error] 187-187: trailing spaces
(trailing-spaces)
[error] 271-271: trailing spaces
(trailing-spaces)
[error] 324-324: trailing spaces
(trailing-spaces)
[error] 328-328: trailing spaces
(trailing-spaces)
[error] 349-349: trailing spaces
(trailing-spaces)
[error] 357-357: trailing spaces
(trailing-spaces)
[error] 364-364: trailing spaces
(trailing-spaces)
[error] 391-391: trailing spaces
(trailing-spaces)
[error] 456-456: no new line character at the end of file
(new-line-at-end-of-file)
Domain Services/Works/Organisation-V1.0.0.yaml
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 588-588: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (8)
Domain Services/Works/Muster-Roll-Service-v1.0.0.yaml (2)
266-267
: Update to ResponseInfo schema reference looks goodThe update to the
ResponseInfo
schema reference is consistent with the earlier change toRequestInfo
. This change is approved, but please ensure it's covered by the verification process mentioned in the previous comment about the new common contract version.
242-243
: Verify the new common contract versionThe update to the
RequestInfo
schema reference and the minor description change look good. However, it's important to ensure that the new version of the common contract (common-contract_v1-1.yml
) is compatible with the current implementation.Please run the following script to verify the compatibility:
Also applies to: 254-254
✅ Verification successful
Common contract version verified successfully
The
RequestInfo
schema reference has been updated and is compatible with the current implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the compatibility of the new common contract version # Test: Check if the new common contract version exists and compare its structure with the old version curl -s https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml > new_contract.yml curl -s https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract.yml > old_contract.yml echo "Differences in RequestInfo schema between old and new versions:" diff <(yq '.components.schemas.RequestInfo' old_contract.yml) <(yq '.components.schemas.RequestInfo' new_contract.yml) rm new_contract.yml old_contract.ymlLength of output: 773
Script:
#!/bin/bash # Description: Verify the compatibility of the new common contract version without using yq # Download the new and old contract YAML files curl -s https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml -o new_contract.yml curl -s https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract.yml -o old_contract.yml echo "Extracting RequestInfo schema from old_contract.yml" grep -A 20 "RequestInfo:" old_contract.yml > old_requestinfo.yml echo "Extracting RequestInfo schema from new_contract.yml" grep -A 20 "RequestInfo:" new_contract.yml > new_requestinfo.yml echo "Differences in RequestInfo schema between old and new versions:" diff old_requestinfo.yml new_requestinfo.yml # Clean up temporary files rm new_contract.yml old_contract.yml old_requestinfo.yml new_requestinfo.ymlLength of output: 2847
Domain Services/Works/Estimate-service-v1.0.0.yaml (5)
30-31
: Improved consistency in schema referencesThe updates to the
$ref
links across multiple locations in the file are consistent and point to the correct common schema definitions. This change enhances the maintainability and standardization of the API specification.Also applies to: 57-58, 71-72, 157-158, 254-255, 265-266, 399-400, 430-431, 443-444
35-37
: Improved YAML formatting for multiline descriptionsThe use of the folded style (>) for multiline descriptions enhances the readability of the YAML file while maintaining the content. This change is a good practice for managing longer text in YAML.
Also applies to: 62-64, 145-147
272-277
: Verify required properties for Estimate schemaThe current list of required properties for the
Estimate
schema includes: tenantId, name, address, estimateDetails, and projectId. While these seem appropriate for creating a new estimate, consider the following:
- Are all these properties necessary for update operations as well?
- Should
address
be a required field in all cases?- Is
estimateDetails
always required, or could there be scenarios where an estimate is created with minimal information and details are added later?Please review and confirm if these required properties are appropriate for all use cases of the Estimate schema. If not, consider making some properties optional or implementing conditional requirements based on the operation type (create vs. update).
358-363
: Logical structure for EstimateDetail required propertiesThe required properties for the
EstimateDetail
schema are well-defined. The structure ensures that:
amountDetail
is always provided.- Either a custom
name
or asorId
(schedule of rate ID) is provided, but not necessarily both.This approach provides flexibility while ensuring that essential information is included in each estimate detail. It accommodates both custom items and standard schedule of rate items effectively.
392-393
: Appropriate required properties for AmountDetail and EstimateRequestThe updates to the required properties for both
AmountDetail
andEstimateRequest
schemas are appropriate:
- For
AmountDetail
, settingamount
as required ensures that essential financial information is always provided.- For
EstimateRequest
, requiringRequestInfo
,estimate
, andworkflow
ensures that all necessary components are included in every request.These changes contribute to more robust and complete API requests and responses.
Also applies to: 422-424
Domain Services/Works/Organisation-V1.0.0.yaml (1)
418-421
: Approve date format changes in OrgSearchCriteriaThe update of
createdFrom
andcreatedTo
formats frominteger
tolong
in the OrgSearchCriteria schema is appropriate. Usinglong
for date ranges in search criteria is consistent with best practices and allows for precise timestamp representation.This change improves the accuracy of date-based searches and is consistent with common API design patterns.
RequestInfo: | ||
$ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/ResponseInfo' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect schema reference in RequestInfoWrapper
There's an error in the schema reference for RequestInfoWrapper
. It's currently pointing to ResponseInfo
instead of RequestInfo
.
Please apply the following fix:
- $ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/ResponseInfo'
+ $ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/RequestInfo'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
RequestInfo: | |
$ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/ResponseInfo' | |
RequestInfo: | |
$ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/RequestInfo' |
- description: >- | ||
All create events will be added to this topic. Persister and Indexer | ||
will read the events from it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify description for update-estimate schema
The descriptions for save-estimate
and update-estimate
schemas provide valuable information about the event-driven nature of the system. However, there appears to be an error in the update-estimate
description:
update-estimate:
allOf:
- $ref: '#/components/schemas/EstimateRequest'
- description: >-
All create events will be added to this topic. Persister and Indexer
will read the events from it.
The description for update-estimate
mentions "create events," which is likely incorrect for an update operation.
Please update the description for update-estimate
to accurately reflect update events:
update-estimate:
allOf:
- $ref: '#/components/schemas/EstimateRequest'
- description: >-
- All create events will be added to this topic. Persister and Indexer
+ All update events will be added to this topic. Persister and Indexer
will read the events from it.
Also applies to: 454-456
description: >- | ||
Successful response sorted by reverse chronological order of | ||
creation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolved typo in response description
The typo in the response description noted in a previous review has not been addressed. The description should be updated for clarity.
Please update the description as follows:
- description: >-
- Successful response sorted by reverse chronological order of
- creation
+ description: Successfully retrieved estimates sorted by reverse chronological order of creation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
description: >- | |
Successful response sorted by reverse chronological order of | |
creation | |
description: Successfully retrieved estimates sorted by reverse chronological order of creation. |
format: double | ||
validTo: | ||
description: Validity to date of registration in epoch time. | ||
type: number | ||
format: integer | ||
format: double |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revise date formats in Function schema
The change of validFrom
and validTo
formats from integer
to double
in the Function schema raises the same concerns as with dateOfIncorporation
. Using double
for date representation can lead to precision issues.
Consider changing the format for both validFrom
and validTo
:
validFrom:
description: Validity from date of registration in epoch time
type: number
- format: double
+ format: int64
validTo:
description: Validity to date of registration in epoch time.
type: number
- format: double
+ format: int64
Alternatively, if you need to represent dates with higher precision:
validFrom:
description: Validity from date of registration
- type: number
- format: double
+ type: string
+ format: date-time
validTo:
description: Validity to date of registration
- type: number
- format: double
+ type: string
+ format: date-time
Ensure consistency in date representation across the entire API specification.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
format: double | |
validTo: | |
description: Validity to date of registration in epoch time. | |
type: number | |
format: integer | |
format: double | |
format: int64 | |
validTo: | |
description: Validity to date of registration in epoch time. | |
type: number | |
format: int64 |
format: double | ||
orgAddress: | ||
type: array | ||
items: | ||
$ref: 'https://raw.githubusercontent.com/egovernments/DIGIT-OSS/master/core-services/docs/common-contract_v1-1.yml#/components/schemas/Address' | ||
$ref: '#/components/schemas/Address' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider date format and approve Address schema change
The change of dateOfIncorporation
format from integer
to double
might lead to precision issues when representing dates. It's generally recommended to use either integer
(for Unix timestamps) or string
with date-time
format for dates in OpenAPI specifications.
Consider changing the dateOfIncorporation
format:
dateOfIncorporation:
description: Epoch time representing date of incorporation of organisation
type: number
- format: double
+ format: int64
Alternatively, if you need to represent dates with higher precision:
dateOfIncorporation:
description: Date of incorporation of organisation
- type: number
- format: double
+ type: string
+ format: date-time
The update to reference a local Address schema for orgAddress
is approved, as it improves the specification's self-containment.
Committable suggestion was skipped due to low confidence.
Detailed estimate, measurement book and SOR/rates specs have been reviewed and added to the repository.
Summary by CodeRabbit
New Features
Documentation