forked from ctfdavis/nestjs-mikro-orm-pageable
-
Notifications
You must be signed in to change notification settings - Fork 6
Some refactorings and test work #1
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
Draft
jadejr
wants to merge
49
commits into
emulienfou:main
Choose a base branch
from
jadejr:fork
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ogic :( I don't quite get the logic here, so this could totally be wrong
This could be controversial, but it made the link building code easier to understand for me.
… output I'm not sure if if it's worth doing it this way, but at least it helps the test pass.
Author
|
now all the tests pass again |
Author
|
One thing I noticed is that you can't reasonably use iso dates in queries like |
We might wanna use a real request mock if we need more vars
…ven necessary) This is probably not necessary if i could understand what the most complex valid filter query could look like, then the operator parsing itself could be improved instead.
Currently you have to change the comment out
`app = await NestFactory.create<NestExpressApplication>(ApplicationModule, new ExpressAdapter(), { logger: false });`
and uncomment
`app = await NestFactory.create<NestFastifyApplication>(ApplicationModule, new FastifyAdapter(), { logger: false })`
To complete this, we need to pass a variable to jest to decide which one to use
A major change here is that express v5 requires `.set('query parser', 'extended')` to support the filters
This makes it easier to work with other databases
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is just some random work I did to attempt to make the tests pass and the logic make more sense to me.
I doubt you'll want to accept all of this, but at least the tests are on their way to passing.
page-factory.spec.tsdoes pass, while the decorator tests do not.I can split this out into more useful bits if you're interested.
I'm a bit stuck on making the paginator decorator tests pass because there are too many different names and usages
related to limit, itemsPerPage, and size.
The README suggests that limit should effectively be like pageSize (or itemsPerPage) as many offset paginators use , while the e2e tests treat limit as a total result limit.
I could really use some insight here to finish this off.