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

#15009 bug with not contains #16853

Conversation

juri-sinitson
Copy link

@juri-sinitson juri-sinitson commented Nov 26, 2024

Filter bug fixes

Fixes the #15009 and adding unit tests for filtering.
Feel free to modify my code before merging when necessary.

Additionally move the filter method there
NOTE:
1. Configured jest according to https://thymikee.github.io/jest-preset-angular/docs/getting-started/installation
2. In was recommended in diverse sources to comment out the files
in `tsconfig.spec.json` to avoid conflicts with jest
- Especially the fixing the global filter `notContains`
which is the main scope of this issue.
Copy link

vercel bot commented Nov 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Nov 26, 2024 2:46pm
primeng-v18 ⬜️ Ignored (Inspect) Visit Preview Nov 26, 2024 2:46pm

@juri-sinitson
Copy link
Author

Remarks

  1. I've covered the whole filter method of TableService method and the most of the FilterService
  2. Maybe it's reasonable to link the unit tests of TableService to the documentation of the table filtering

@juri-sinitson
Copy link
Author

Also let me know, if need to change something

@mertsincan
Copy link
Member

Hi @juri-sinitson, thanks a lot for your contribution! Can you please resend your PR for v18 branch? Also, please don't change the folder structure (table.service) or add your IDE setting. We'll update folders after the 19 release.

@mertsincan mertsincan closed this Dec 6, 2024
@juri-sinitson
Copy link
Author

juri-sinitson commented Dec 20, 2024

Hi @mertsincan,

sorry for a big delay. I've noticed that the version 19 is out and it's a part of master by an appropriate tag.

For me it looks like it's now better to resend a PR for master instead of v18.
If I'm wrong just let me know for which branch I should resend the PR. Also let me know if you have some other wishes.

I've just rebased my branch to the current master.
I've also implemented your remarks about not changing the folder structure now and not adding IDE settings.
So I'm going to resend my PR later today.

@juri-sinitson
Copy link
Author

@mertsincan
I've resent the PR here.

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.

2 participants