test: Add branching fixes and other fixes for advance request #2865
Conversation
|
PR title must start with "fix:" or "feat:" or "test:" |
|
PR description must contain a link to a clickup |
|
PR title must start with "fix:" or "feat:" or "test:" |
|
PR description must contain a link to a clickup |
|
PR title must start with "fix:" or "feat:" or "test:" |
1 similar comment
|
PR title must start with "fix:" or "feat:" or "test:" |
bistaastha
changed the title
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested.
- Reviewed the entire pull request up to bf33ef7
- Looked at
469lines of code in5files - Took 2 minutes and 41 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
2additional comments because they didn't meet confidence threshold of50%.
1. src/app/core/services/advance-request.service.ts:72:
- Assessed confidence :
50% - Comment:
The import ofcloneDeepfrom lodash is not used in this file. Consider removing unused imports to keep the code clean and efficient. - Reasoning:
The import ofcloneDeepfrom lodash is not used in theadvance-request.service.tsfile. This is a minor issue but it's always a good practice to remove unused imports as they can lead to confusion and unnecessary increase in the bundle size.
2. src/app/core/services/tasks.service.ts:850:
- Assessed confidence :
100% - Grade:
0% - Comment:
The functionmapScalarAdvanceStatsResponsehas been removed but it is still being used here. This will cause a runtime error. Please replace it with the appropriate function or add themapScalarAdvanceStatsResponsefunction back to the service. - Reasoning:
The functionmapScalarAdvanceStatsResponsehas been removed from theTasksServicebut it is still being used in thegetSentBackAdvanceTasksfunction. This will cause a runtime error when the function is called.
Workflow ID: wflow_Wki3dyRfEexgGI37
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 067c2a0
- Looked at
15lines of code in1files - Took 1 minute and 42 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of50%.
Workflow ID: wflow_7ZTB7JjHIr4O2S9O
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on b53f66d
- Looked at
69lines of code in1files - Took 1 minute and 51 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/fyle/my-advances/my-advances.page.spec.ts:250:
- Assessed confidence :
0% - Comment:
Good job on updating the test case to reflect the changes in the PR. This ensures that the test cases are in sync with the code changes. - Reasoning:
The test case at line 250 has been updated to reflect the changes in the PR. The test case description and the expected result have been updated to include the 'sent back' state of advance requests. This is a good practice as it ensures that the test cases are in sync with the code changes.
Workflow ID: wflow_tOC7A0QuCi4TYy6C
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
❌ Changes requested.
- Performed an incremental review on a564670
- Looked at
125lines of code in3files - Took 3 minutes and 29 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of50%.
Workflow ID: wflow_KHMKM4QPPHYgf6Fw
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 5c0c0dc
- Looked at
219lines of code in4files - Took 3 minutes and 15 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/core/services/advance-request.service.ts:111:
- Assessed confidence :
100% - Grade:
0% - Comment:
Please remove theconsole.logstatement. It's not a good practice to leave console logs in the production code. - Reasoning:
In themapAdvanceRequestmethod, theconsole.logstatement seems to be a leftover from debugging and should be removed. It's not a good practice to leave console logs in the production code as it can expose sensitive information and make the console cluttered.
Workflow ID: wflow_snH8e1I0j1INSyqh
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 4dc45d0
- Looked at
15lines of code in1files - Took 2 minutes and 25 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/core/services/advance-request.service.spec.ts:215:
- Assessed confidence :
100% - Grade:
0% - Comment:
The PR description mentions that the functionmapScalarAdvanceStatsResponsewas removed from/src/app/core/services/tasks.service.ts. However, this change is not reflected in the diff. Please confirm if this change was made. - Reasoning:
The PR description mentions that an unused functionmapScalarAdvanceStatsResponsefrom/src/app/core/services/tasks.service.tswas removed. However, the diff does not show this change. This could be an oversight or a mistake in the PR description.
Workflow ID: wflow_X3ympd4Mo8cXKtHd
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on f341538
- Looked at
262lines of code in2files - Took 1 minute and 41 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/core/mock-data/extended-advance-request.data.ts:595:
- Assessed confidence :
0% - Comment:
Good use of lodash's cloneDeep function to reduce redundancy in mock data. - Reasoning:
The changes in the mock data files are mostly about removing redundant data and using the cloneDeep function from lodash to clone the base object and then modify the necessary properties. This is a good practice as it reduces redundancy and makes the code cleaner and easier to maintain.
Workflow ID: wflow_IIYwzzwr6Akh9iJ4
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 3 days remaining in your trial. Upgrade here.
bistaastha
requested review from
howdysuraj,
suyashpatil78,
Julias0 and
Chethan-Fyle
…mobile-app into FYLE-86cuxuafw-2-test-fixes
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 6fb25ca
- Looked at
13lines of code in1files - Took 1 minute and 7 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/core/mock-data/advance-platform.data.ts:84:
- Assessed confidence :
33% - Comment:
Please add a comment explaining why the 'last_approved_at' date was changed. If this change has any significance in the tests, it should be documented. - Reasoning:
The change in this PR is a modification of the 'last_approved_at' date in the mock data. This change seems to be harmless as it's just a change in the test data. However, it's important to understand why this change was made. If the change in date has any significance in the tests, it should be documented in the PR or in the code comments.
Workflow ID: wflow_WlVnbydD80rfSrc6
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 2 days remaining in your trial. Upgrade here.
There was a problem hiding this comment.
👍 Looks good to me!
- Performed an incremental review on 57c98f6
- Looked at
14lines of code in1files - Took 1 minute and 32 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
1additional comments because they didn't meet confidence threshold of50%.
1. /src/app/fyle/my-advances/my-advances.page.ts:197:
- Assessed confidence :
50% - Comment:
Please confirm if the 'areq_state' can indeed be 'INQUIRY' and if this condition is necessary. - Reasoning:
The changes in the PR seem to be about adding a new condition for the 'sentBackAdvance' variable. The new condition checks if the 'areq_state' is 'INQUIRY'. This seems to be a logical change and doesn't appear to introduce any bugs. However, I would like to confirm if the 'areq_state' can indeed be 'INQUIRY' and if this condition is necessary.
Workflow ID: wflow_TZOH6DbdgpR57UTR
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
⌛ You have 2 days remaining in your trial. Upgrade here.
bistaastha
changed the title
|
Summary:
This PR enhances the
AdvanceRequestServiceto handle different states of advance requests, adds corresponding mock data, updates unit tests, reflects these changes in a new test file, removes an unused function, updates thelast_approved_atdate inadvancePlatformmock data, and updates the condition forsentBackAdvanceto handle 'INQUIRY' state.Key points:
AdvanceRequestServiceto handle different states of advance requests./src/app/core/mock-data/extended-advance-request.data.tsand/src/app/core/mock-data/platform/v1/advance-request-platform.data.ts./src/app/core/services/advance-request.service.spec.ts./src/app/fyle/my-advances/my-advances.page.spec.ts.mapScalarAdvanceStatsResponsefrom/src/app/core/services/tasks.service.ts.last_approved_atdate inadvancePlatformmock data in/src/app/core/mock-data/advance-platform.data.ts.sentBackAdvancein/src/app/fyle/my-advances/my-advances.page.tsto handle 'INQUIRY' state.Generated with ❤️ by ellipsis.dev