baseline icon indicating copy to clipboard operation
baseline copied to clipboard

517 generic mapper

Open Manik-Jain opened this issue 2 years ago • 4 comments

Description

Adding a Generic mapper to convert object to Type at runtime

Related Issue

https://github.com/eea-oasis/baseline/issues/517

Motivation and Context

We need a general purpose mapper to convert object to Type at runtime, with capabilities to define some explicit properties on the type

How Has This Been Tested

Screenshots (if appropriate)

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [X ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Request to be added as a Code Owner/Maintainer

Checklist

  • [X] My code follows the code style of this project.
  • [X] My change requires a change to the documentation.
  • [X] I have updated the documentation accordingly.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.
  • [X] I commit to abide by the Responsibilities of Code Owners/Maintainers.

Manik-Jain avatar Sep 25 '22 04:09 Manik-Jain

Hey @Manik-Jain , what is the status with this one? I believe it is not mergeable until the issue leading to commented tests is resolved. Do you think it would make sense that we fall back to a simple mapper doing something like object.assign while we wait?

ognjenkurtic avatar Oct 05 '22 10:10 ognjenkurtic

Hey @Manik-Jain , what is the status with this one? I believe it is not mergeable until the issue leading to commented tests is resolved. Do you think it would make sense that we fall back to a simple mapper doing something like object.assign while we wait?

I agree. Let's keep things simple and moving. We can always come back to this in the next iteration @Manik-Jain

Therecanbeonlyone1969 avatar Oct 05 '22 21:10 Therecanbeonlyone1969

@Therecanbeonlyone1969 Yeah, we can circle back to this PR once the repository is properly configured. I see that the test folder provided by Nest hasn't been utilized for the purpose of adding unit test suites, which is why the compilation isn't adding anything to dist directory. This needs to be

@ognjenkurtic I gave it a thought, and we can use Object.assign at the cost of type-checking. This can be used temporarily while I fix the test case configuration

Manik-Jain avatar Oct 06 '22 05:10 Manik-Jain

@Therecanbeonlyone1969 Yeah, we can circle back to this PR once the repository is properly configured. I see that the test folder provided by Nest hasn't been utilized for the purpose of adding unit test suites, which is why the compilation isn't adding anything to dist directory. This needs to be

@ognjenkurtic I gave it a thought, and we can use Object.assign at the cost of type-checking. This can be used temporarily while I fix the test case configuration

@Manik-Jain Repository is properly configured when it comes to testing. It is following a common pattern for unit and e2e tests. From NestJs documentation:

Unit tests: Keep your test files located near the classes they test. Testing files should have a .spec or .test suffix.

E2E tests: Keep your e2e test files inside the test directory. The testing files should have a .e2e-spec suffix.

TLDR; Test directory is meant for E2E tests as they are testing the whole scope of the application. Unit tests are next to the things they are testing as this is the scope they are interested in. I would suggest we continue using this approach unless there is some exceptional thing we need to consider.

re: Object.assign. Great, do you want to do that as part of another PR? If yes, please feel free to open an issue.

ognjenkurtic avatar Oct 06 '22 09:10 ognjenkurtic

@Therecanbeonlyone1969 Yeah, we can circle back to this PR once the repository is properly configured. I see that the test folder provided by Nest hasn't been utilized for the purpose of adding unit test suites, which is why the compilation isn't adding anything to dist directory. This needs to be @ognjenkurtic I gave it a thought, and we can use Object.assign at the cost of type-checking. This can be used temporarily while I fix the test case configuration

@Manik-Jain Repository is properly configured when it comes to testing. It is following a common pattern for unit and e2e tests. From NestJs documentation:

Unit tests: Keep your test files located near the classes they test. Testing files should have a .spec or .test suffix.

E2E tests: Keep your e2e test files inside the test directory. The testing files should have a .e2e-spec suffix.

TLDR; Test directory is meant for E2E tests as they are testing the whole scope of the application. Unit tests are next to the things they are testing as this is the scope they are interested in. I would suggest we continue using this approach unless there is some exceptional thing we need to consider.

re: Object.assign. Great, do you want to do that as part of another PR? If yes, please feel free to open an issue.

Hi @ognjenkurtic @Therecanbeonlyone1969

As discussed during the weekly standup, I made a final attempt to fix the failing test cases with reflect TS compiler, and the test cases are all passing now. So, we don't need to move to a new approach as this one works with proper type-safety.

Thanks

Manik-Jain avatar Oct 10 '22 06:10 Manik-Jain

https://automapperts.netlify.app/docs/nestjs

thanks for this @skosito This looks useful. Let me spend some time with it

Manik-Jain avatar Oct 13 '22 13:10 Manik-Jain