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

#260 - Merge Hotfix v1.14.1 into Release v1.15.0 #3885

Merged
merged 20 commits into from
Nov 5, 2024

Conversation

sh16011993
Copy link
Collaborator

@sh16011993 sh16011993 commented Nov 5, 2024

The following are achieved in this hotfix:

bidyashish and others added 20 commits October 17, 2024 15:03
- Changed restriction code `SINF` to `SINR`
- Updated the activity page accordion code to `SINR`
- Created a DB migration to change the code of the `SINF` to `SINR`.
- Renamed `SINF` to `SINR` in the existing `RestrictionCode` enum.
- Changed an existing E2E to ensure `SINF` will be mapped to `LGCY`. 
- Replaced "TREE" with "SINF" in file
"SFAS-TO-SIMS-2024MAR07-LEGACY-RESTRICTIONS.txt" to ensure `SINF`
mapping to `LGCY` for this PR.

### Rollback evidence screenshot

![image](https://github.com/user-attachments/assets/2002ad83-4af3-4ab5-9b9c-3226e02b28e8)

Screenshot of the SINR code on the activity page accordion

![image](https://github.com/user-attachments/assets/c1b4028c-c92b-447a-b417-a10fc8c75f04)

Screenshot of the `sfas_student` table having two records after
restrictions are imported

![image](https://github.com/user-attachments/assets/1b0d2812-3944-4a15-9641-fabe894cbcf4)

Screenshot of the `student_restrictions` table having only one record
with restriction_id = 72 (LGCY)

![image](https://github.com/user-attachments/assets/20e3396d-96db-4190-941a-9e597f945ba8)

Screenshot of the completed job for processing the SINR record on Bull
Dashboard

![image](https://github.com/user-attachments/assets/50db6edd-3070-4e25-80c3-db1ff1a60be0)
The error was possible to be reproducible in the local development
machine using 17Kb files. Any file larger than 16Kb causes the error,
which means that the default chunk size allocated for the stream buffer
was exceeded.

The readable streams have two reading modes: [flowing and
paused](https://nodejs.org/api/stream.html#two-reading-modes).
The connection to `clamav` server was open but was not receiving data
for certain files, which led to the timeout. When the
`passthroughStream.resume();` was added it resulted in the data being
send immediately.
The `passthroughStream.readableFlowing` was defined as null, which means
the below ([source](https://nodejs.org/api/stream.html#three-states))
- readable.readableFlowing === null
- readable.readableFlowing === false
- readable.readableFlowing === true

In the [example from the clamscan](passthroughStream) package, the
`passthrough` stream is piped to an output stream which also resumes the
stream, as mentioned below.

> All
[Readable](https://nodejs.org/api/stream.html#class-streamreadable)
streams begin in paused mode but can be switched to flowing mode in one
of the following ways:
> 
> Adding a ['data'](https://nodejs.org/api/stream.html#event-data) event
handler.
> Calling the
[stream.resume()](https://nodejs.org/api/stream.html#readableresume)
method.
> Calling the
[stream.pipe()](https://nodejs.org/api/stream.html#readablepipedestination-options)
method to send the data to a
[Writable](https://nodejs.org/api/stream.html#class-streamwritable).

Local test results:
 - a 4MB file is processed in less than 3 seconds.
- a 15 MB file is processed in approximately 15 seconds.
- queued 50 files of 15 MB and it had a total processing time of 15
minutes with no errors.

No timeouts adjusted in this PR and we may not adjust any if the tests
on Openshift go well for this change.
The target of this change is to fix the current production issue only.
…es (#3835)

***Acceptance Criteria***
- [x] Jobs must continue processing remaining records in a file after it
fails to process a record.
- [x] SIN and CRA response files will need to be archived after
processing even where there are failed records. (IMB staff will need to
monitor the jobs to confirm if errors occur processing SIMS response
files and address them accordingly)
- [x] Disable error reporting when SIMS Reference_IDX fails to match.
- [x] If records are identified as SIMS records (based on the project
area information being present), errors will be generated as usual.
- [x] If records are identified as SFAS records (based on the project
area information NOT being present), records should be skipped.
- [x] For the SIN validation ([source
reference](https://github.com/bcgov/SIMS/blob/8864eaa8a5f62360f5e3c91c6de06f319e008250/sources/packages/backend/libs/integrations/src/services/sin-validation/sin-validation.service.ts#L119)),
if the `referenceIndex` is not found on SIMS, assume the record is from
SFAS and do not log as an error.
- [x] For income verifications, the records should already be skipped.
Execute a manual test to ensure that a file with records that do not
contain the `VERIFICATION_ID` in the free project area is skipped.
Consider creating an E2E to execute the test.
- [x] Demo for both cases with a sample file.
## Cherry pick commits

The following commits are cherry picked and committed into
`file-integration-fix/cherry-pick-branch` branch from `main` branch.

- #3833 - SIN Validation - Gender Field Limit - Commit:8040205 - SHA:
8040205
- #3831 - SINF restriction bridge mapping and rename to SINR -
Commit:3ed8c2e - SHA: 3ed8c2e
- #3825 - Virus Scanning - Not functioning PROD - Commit: 5bb5050 - SHA:
5bb5050
- #3745 - Modify process that reads SIN & CRA verification response
files - Commit: 20fcda4 - SHA: 20fcda4
…le displays when "Validate" or "Create now" are selected for valid bulk upload (#3860)

In Vuetify newer versions there's a [change on how VFileInput deals with
the model
value](https://vuetifyjs.com/en/api/v-file-input/#props-model-value):
- If `multiple` prop is undefined or false, VFileInput will return a
`File`. If true, it returns a `File[]`.
 
Instead of destructuring a file from an array, the model file can be
used as the blob required.
When starting a socket to send a file content to `clamav` using the
passthrough and no content is sent, the connection stays open for an
indefinite amount of time (over 5 minutes).
Changed queue-consumers to throw an error and remove the job, **same
behavior when the file is not found**.

_We can have a separate ticket to apply validation for the file upload
to avoid accepting empty files, but I considered it outside this hotfix
issue._

ClamAV debug log when a empty file is send for scanning.
```
2024-10-31 08:32:55 node-clam: Initially testing socket/tcp connection to clamscan server.
2024-10-31 08:32:55 node-clam: Attempting to establish socket/TCP connection for "_ping"
2024-10-31 08:32:55 node-clam: using remote server: 172.18.0.6:3310
2024-10-31 08:32:55 node-clam: Established connection to clamscan server!
2024-10-31 08:32:55 node-clam: PONG!
2024-10-31 08:32:55 node-clam: Established connection to clamscan server!
2024-10-31 08:32:55 node-clam: Done with the full pipeline.
2024-10-31 08:32:55 node-clam: Socket/Host connection closed.
2024-10-31 08:33:00 node-clam: ClamAV has been scanning for 5 seconds...
...
2024-10-31 08:35:25 node-clam: ClamAV has been scanning for 150 seconds...
...
2024-10-31 08:45:30 node-clam: ClamAV has been scanning for 755 seconds...
...
```
…equest file (#3864)

**AC*
The expected file name is just BC####.OS in all environments. 

- [x] Do not use value from environment variable
`ESDC_ENVIRONMENT_CODE`.
- [x] No changes needed to response file handling.

Need to fix Invalid header format 0010000000120241023BC. There’s 8
characters, but should be 6 characters only.

- [x]  Update batch number to "6" from "8"

https://github.com/bcgov/SIMS/blob/main/sources/packages/backend/libs/integrations/src/esdc-integration/sin-validation/sin-validation-files/sin-validation-file-header.ts#L38
- [x]  Fix e2e tests

*Note:
No E2E file changed [PCSLP.PBC.BC0000.ISR
](https://github.com/bcgov/SIMS/blob/8faa4e583c4d0ce8a657be52e9965bd2ed8c3caa/sources/packages/backend/apps/queue-consumers/src/processors/schedulers/esdc-integration/sin-validation-integration/_tests_/sin-receive-files/PCSLP.PBC.BC0000.ISR)
Current E2E test has correct header format `00100500220220921BC1 ` -> 20
Character
 Sample Invalid header `0010000000120241023BC`  -> 22 Character
Same fix done for the bulk upload in
#3860.
Acceptance Criteria
- [x] Update CRA Response Integration job to search for new file name 
    - [x] [CRA Environment Code]BCSA#####.TXT
- [x] No change on request - confirmed with IMB CRA income verification
request file name can remain the same.

**Context:** 
DEV: ABCSA#####.TXT
TEST: ABCSA#####.TXT
PROD: PBCSA#####.TXT

**Current File Names for CRA Income:** 
- CCRA_REQUEST_${this.craConfig.environmentCode}${sequenceFile}.DAT
- /CCRA_RESPONSE_[\w]*\.txt/i

*note
Regex test

```
const environmentCode = 'A';
const regex = new RegExp(`[${environmentCode}]BCSA\\d{5}\\.TXT`, 'i');
const testString = 'ABCSA00001.TXT';

console.log(regex.test(testString)); // true
```
## SFAS Import update on NEB

- [x] NEB: Logic for parsing NEB updated to consider the last 2
characters as decimals.
- [x] BCGG: Verified that the logic for parsing BCGG is not considering
decimals.(No code change here)
- [x] Verified the part time applications in import are **NOT**
considering decimals during the parsing of amounts.
- [x] Verified the full time applications in import are considering last
2 characters as decimals during the parsing of amounts.

## E2E Tests

- [x] Adjusted the E2E tests according to updated NEB logic.

## Manual Test

Imported records with NEB successfully


![image](https://github.com/user-attachments/assets/75b163bd-090d-4a61-95df-8cea4b496255)
Added a gap to skip some sequence numbers while generating the MSFAA
sequence to avoid conflicts with legacy MSFAA.
@sh16011993 sh16011993 added the Hotfix PR created for hotfix label Nov 5, 2024
@sh16011993 sh16011993 self-assigned this Nov 5, 2024
Copy link

sonarqubecloud bot commented Nov 5, 2024

Copy link

github-actions bot commented Nov 5, 2024

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 22.36% ( 3694 / 16524 )
Methods: 10.33% ( 212 / 2053 )
Lines: 25.67% ( 3204 / 12482 )
Branches: 13.98% ( 278 / 1989 )

Copy link

github-actions bot commented Nov 5, 2024

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 65.43% ( 583 / 891 )
Methods: 59.26% ( 64 / 108 )
Lines: 68.54% ( 464 / 677 )
Branches: 51.89% ( 55 / 106 )

Copy link
Collaborator

@bidyashish bidyashish left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

github-actions bot commented Nov 5, 2024

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 85.92% ( 1220 / 1420 )
Methods: 87.86% ( 123 / 140 )
Lines: 86.83% ( 1035 / 1192 )
Branches: 70.45% ( 62 / 88 )

Copy link

github-actions bot commented Nov 5, 2024

E2E SIMS API Coverage Report

Totals Coverage
Statements: 66.01% ( 5571 / 8439 )
Methods: 63.3% ( 683 / 1079 )
Lines: 70.12% ( 4401 / 6276 )
Branches: 44.93% ( 487 / 1084 )

@andrewsignori-aot andrewsignori-aot changed the title Hotfix/v1.14.1 #260 - Merge Hotfix v1.14.1 into Release v1.15.0 Nov 5, 2024
@andrewsignori-aot andrewsignori-aot added the Branch Sync-up Branch features move between main/release/hotfix branches. label Nov 5, 2024
Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Branch Sync-up Branch features move between main/release/hotfix branches. Hotfix PR created for hotfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants