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.

Sign up for GitHub

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

Jump to bottom

SER 2543: Make blank?, present?, and any? work for AF query objects #68

Merged
merged 8 commits into from
Sep 28, 2023
Merged

SER 2543: Make blank?, present?, and any? work for AF query objects #68

merged 8 commits into from
Sep 28, 2023

Conversation

kencorley
Copy link
Contributor

No description provided.

@kencorley
Copy link
Contributor Author

Make .blank? and .present? work in ActiveForce::ActiveQuery objects

TABeauchat
TABeauchat approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@TABeauchat TABeauchat left a comment

Choose a reason for hiding this comment

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

Nice and simple addition!

@sbelknap-bf
Copy link
Contributor

looks good, i would also consider adding .any? to the list of delegators... as that is the method that comes to mind for me when i want to check if there are any results to a query... whereas .blank? and .present? can be called on any Object, and may have previously been cause for confusion previously... because you could call .present? on an ActiveQuery Object and get true, even if the ActiveQuery Object contained no records. Same thing for .blank?. But if I try calling .any? on an ActiveQuery Object, I will get a no method error

@kencorley
Copy link
Contributor Author

looks good, i would also consider adding .any? to the list of delegators... as that is the method that comes to mind for me when i want to check if there are any results to a query... whereas .blank? and .present? can be called on any Object, and may have previously been cause for confusion previously... because you could call .present? on an ActiveQuery Object and get true, even if the ActiveQuery Object contained no records. Same thing for .blank?. But if I try calling .any? on an ActiveQuery Object, I will get a no method error

Added.

@kencorley kencorley changed the title SER 2543: Make blank and present work for AF query objects SER 2543: Make blank?, present?, and any? work for AF query objects Sep 28, 2023
sbelknap-bf
sbelknap-bf approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@sbelknap-bf sbelknap-bf left a comment

Choose a reason for hiding this comment

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

thanks for adding my .any? :) looks good

asedge
asedge approved these changes Sep 28, 2023
View reviewed changes
Copy link
Collaborator

@asedge asedge left a comment

Choose a reason for hiding this comment

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

Nice work, thanks!

@asedge asedge merged commit 11a9e10 into main Sep 28, 2023
2 checks passed
@asedge asedge deleted the 2543-make-blank-and-present-work-for-af-query-objects branch September 28, 2023 21:13
salonishah23
salonishah23 reviewed Oct 2, 2023
View reviewed changes
Copy link

@salonishah23 salonishah23 left a comment

Choose a reason for hiding this comment

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

Good adds but it would be nice to understand where present? is used and whether that usage can be replaced with any? or exists? for an easy win

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salonishah23 salonishah23 salonishah23 left review comments

@sbelknap-bf sbelknap-bf sbelknap-bf approved these changes

@TABeauchat TABeauchat TABeauchat approved these changes

@asedge asedge asedge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kencorley @sbelknap-bf @asedge @TABeauchat @salonishah23