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

Updates npm packages of lib microservice #112

Conversation

OliverGeneser
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2023

Codecov Report

Merging #112 (5e98069) into feature/distributed-demo (3e1935e) will decrease coverage by 0.29%.
Report is 1 commits behind head on feature/distributed-demo.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@                     Coverage Diff                      @@
##           feature/distributed-demo     #112      +/-   ##
============================================================
- Coverage                     66.66%   66.38%   -0.29%     
============================================================
  Files                            42        7      -35     
  Lines                           537      119     -418     
  Branches                         31        4      -27     
============================================================
- Hits                            358       79     -279     
+ Misses                          160       37     -123     
+ Partials                         19        3      -16     
Files Coverage Δ
...rs/lib/src/files/services/files-service.factory.ts 71.42% <ø> (ø)
...ers/lib/src/files/services/gitlab-files.service.ts 95.23% <ø> (ø)
...vers/lib/src/files/services/local-files.service.ts 22.22% <ø> (ø)

... and 36 files with indirect coverage changes

Components Coverage Δ
Website ∅ <ø> (∅)
Lib Microservice 66.38% <ø> (-3.53%) ⬇️

@prasadtalasila
Copy link
Contributor

prasadtalasila commented Sep 21, 2023

@OliverGeneser Thanks for the PR.
Can you please identify the vulnerability that you fixed through this PR? Please check with dependabot / snyk to see if some licenses still have these problems.

Please do check for different versions of transitive dependencies to make sure that they are of same version. The npm seems might have a way to enforce use of same dependency version in all dependencies. Can you check if it helps?

Please do also check the licenses for the all the packages to see if the packages can be used with GPL Version 3 used by DTaaS software. This github resource might help.

@prasadtalasila
Copy link
Contributor

Please check the commands given in issue #109 to see if the vulnerabilities have been taken care of.

@OliverGeneser OliverGeneser changed the title Updated all backward-compatible packages Updated all packages Sep 25, 2023
@OliverGeneser
Copy link
Contributor Author

OliverGeneser commented Sep 25, 2023

@prasadtalasila This PR fixes 47 vulnerabilities and most relates to a DOS vulnerability in semver and word-wrap or a Resource Consumption vulnerability in graphql
audit-output.txt

@prasadtalasila prasadtalasila changed the title Updated all packages Updated npm packages of lib microservice Sep 26, 2023
@prasadtalasila prasadtalasila changed the title Updated npm packages of lib microservice Updates npm packages of lib microservice Sep 26, 2023
Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

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

Please check the comments posted on issue #109 as well. Thanks.

@@ -8,7 +8,9 @@ import { IFilesService } from "../interfaces/files.service.interface";
@Injectable()
export default class LocalFilesService implements IFilesService {
// eslint-disable-next-line no-useless-constructor
constructor(private configService: ConfigService) {}
constructor(private configService: ConfigService) {
// Empty constructor
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this comment line added?

"@nestjs/platform-express": "^10.2.6",
"axios": "^1.5.0",
"dotenv": "^16.3.1",
"eslint-plugin-import": "^2.28.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

These two eslint packages can go into devDependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

The major changes seem to be:


   "dependencies": {  
     "@apollo/server": "^4.9.3",  
     "express": "^4.17.1", 
     "graphql-scalars": "^1.22.2", 
     "mock-fs": "^5.2.0", 
     "reflect-metadata": "^0.1.13", 
     "type-graphql": "^2.0.0-beta.2" 
   }, 
   "devDependencies": { 
     "supertest": "^6.1.3", 
     "ts-node": "^10.0.0", 
   }, 
   "peerDependencies": { 
     "graphql-scalars": "^1.22.2" 
   }

These packages are new additions. Why do we need them?

Also the graphql-scalars is in two places - devDependencies and peerDependencies. If needed, please add it at the right place.

"@types/jest": "^29.5.3",
"@types/node": "18.11.18",
"@types/jest": "^29.5.5",
"@types/node": "20.6.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

can go to 20.7.0

@prasadtalasila
Copy link
Contributor

@OliverGeneser , please rebase your code to the latest commit on the feature/distributed-demo branch.

I see two changes in the packages.

  1. Addition of graphql-scalars. Can you please clarify the reason for adding this?
  2. Replacement of apollo-server-express with @apollo/server. Why is it done?

@OliverGeneser
Copy link
Contributor Author

@prasadtalasila
Graphql-scalars is added because it is a peer dependency for type-graphql
Apollo-server-express is replaced with @apollo/server because it is deprecated

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 15310 lines exceeds the maximum allowed for the inline comments feature.

@prasadtalasila
Copy link
Contributor

@OliverGeneser, using any version is vague. Please change the grapgql-scalars to the latest compatible version.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 15309 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Oct 2, 2023

Code Climate has analyzed commit 5e98069 and detected 0 issues on this pull request.

View more on Code Climate.

@prasadtalasila prasadtalasila merged commit 5aa755d into INTO-CPS-Association:feature/distributed-demo Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants