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

OData lambda style filter query #3155

Merged
merged 24 commits into from
Nov 4, 2024

Conversation

Tanddant
Copy link
Contributor

@Tanddant Tanddant commented Oct 21, 2024

Category

  • Bug fix?
  • New feature?
  • New sample?
  • Documentation update?

Related Issues

Based off the discussion over on #2781

What's in this Pull Request?

This PR is not completely done, but I needed a place to track the final changes (and docs) based off the input in #2781

  • We like "method 1", seems simpler
  • Drop "Field" from all the names so Lookup() vs LookupField().
  • switch to camel case so Lookup becomes lookup()
  • could simplify the names, equal instead of EqualsTo and notEqual vs NotEqualTo
    • I'm still open to the idea of having even shorter values, such as .eq(...) alongside the others, would like opinions
  • Swap and/or for All/Some, just seems clearer to someone showing up with no knowledge of the filter methods
  • pass in multiple objects you can use ...[] to gather them into an array within the method.
  • this should be able to support all the filter cases, another reason to drop the "Field" name from the builder functions. Like for a site if you give it ISiteInfo you could filter on Text("Title").EqualsTo("blah");
    • The following is tested to work
    sp.web.lists.filter<IListInfo>(f => f.text("Title").equal("Employees"))

I'll happily start updating the docs once we have a "somewhat sign-off" on the above been done to an acceptable state 😊

bcdmi and others added 21 commits August 19, 2023 08:09
This solved the issue where if you reused the same variable name in your long queries it broke
down to one warning which I can't resolve
# Conflicts:
#	package-lock.json
#	packages/sp/spqueryable.ts
@Tanddant Tanddant changed the title V4 o data filter query OData lambda style filter query Oct 21, 2024
@Tanddant
Copy link
Contributor Author

Latest commit added "auto typing" for things like lists, webs, and so on, where we "know the type"

I.e

await sp.web.webinfos.filter(w => w.text("Title").equal('MyWebTitle'))();

Without having to specify that Title is a field on lists

I haven't tested this in every possible combination, but it looks to be working without breaking the list item stuff, should greatly improve the usability 😊

@bcameron1231
Copy link
Collaborator

Thanks! Will be doing some testing on this.

@Tanddant
Copy link
Contributor Author

@bcameron1231 Let me know if you have any issues, I have some initial work on the first draft of the docs, do you want them here, or in a separate PR?

@bcameron1231
Copy link
Collaborator

Let's get those in the same commit. It will also help me test correctly. 😁 Excited to see this.

Can you also try and squash your commits?

We like "method 1", seems simpler
Drop "Field" from all the names so Lookup() vs LookupField().
switch to camel case so Lookup becomes lookup()
could simplify the names, equal instead of EqualsTo and notEqual vs NotEqualTo
Swap and/or for All/Some, just seems clearer to someone showing up with no knowledge of the filter methods
pass in multiple objects you can use ...[] to gather them into an array within the method.
this should be able to support all the filter cases, another reason to drop the "Field" name from the builder functions. Like for a site if you give it ISiteInfo you could filter on Text("Title").EqualsTo("blah");
@Tanddant
Copy link
Contributor Author

So this might just be my git skills that are lacking - I can't seem to squash the old commits without ending up with like 300 merge confilts that need manual processing, but I at least got the newest ones squashed into two

@bcameron1231
Copy link
Collaborator

Thank you. I've done some testing, it looks to be working. However it looks likes the plurals may be gone? i.e, .equal() vs .equals(). We'd like the plurals back 👍

Here is what we plan to do once you make those changes. Given this has backwards compatibility, we're going to get this merged in and try to push it out in November. Our goal is to promote this new functionality in the next Month's call as a beta functionality.

Our hope is that we can get others to start using it and gather some feedback on the implementation. Maybe we could get you set up to do a Demo on a call at some point to explain how it works.

@Tanddant
Copy link
Contributor Author

I think the rename was just down to misunderstanding one of the last feedback items 😊 - changed it up for .equals .notEquals() .greaterThanOrEquals() .lessThanOrEquals()

Looking forward to getting some real world feedback once it hits beta!

@juliemturner juliemturner added this to the 4.7.0 milestone Oct 30, 2024
@bcameron1231 bcameron1231 merged commit a6611e0 into pnp:version-4 Nov 4, 2024
1 check failed
@bcameron1231
Copy link
Collaborator

🚀🚀🚀🚀🚀🚀

@Tanddant
Copy link
Contributor Author

Tanddant commented Nov 4, 2024

Are the lint rules different on the runs here and the settings in the project? 😅 - I only had one warning locally, which I couldn't get rid of 👀

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.

4 participants