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

[Core] New Filters and Logic #9548

Merged
merged 6 commits into from
Feb 18, 2025
Merged

[Core] New Filters and Logic #9548

merged 6 commits into from
Feb 18, 2025

Conversation

ridz1208
Copy link
Collaborator

@ridz1208 ridz1208 commented Feb 4, 2025

Brief summary of changes

This PR creates new filters and modifies existing ones to add functionality and make them more scalable. Two new composite filters can also be used to join other filters in an AND or OR logic. These filters should allow us to move away from writing a new filter with "combined" functionality for each module (i.e. UserProjectMatchOrHasAnyPermissions) instead you can use independent filters and join them in the order you need them to be.

Changes in PR #9549 demo the use of these filters and how to combine them

PS these changes should not break existing code

Testing instructions (if applicable)

  1. Hard to test on it's own but will be tested buy changes in another PR

Link(s) to related issue(s)

  • Resolves # (Reference the issue this fixes, if any.)

@ridz1208 ridz1208 added Category: Feature PR or issue that aims to introduce a new feature Beginner Friendly PR or Issue appears to be easy for someone to use to familiarize themselves with LORIS Priority: High PR or issue should be prioritised over others for review and testing Proposal PR or issue suggesting an improvement that can be accepted, rejected or altered Language: PHP PR or issue that update PHP code Project: C-BIG Issue or PR related to the C-BIG project Difficulty: Simple PR or issue that should be easy to implement, review, or test Category: Refactor PR or issue that aims to improve the existing code labels Feb 4, 2025
@ridz1208 ridz1208 added this to the 27.0.0 milestone Feb 13, 2025
Copy link
Contributor

@skarya22 skarya22 left a comment

Choose a reason for hiding this comment

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

Few comments after code review, testing now

// phan only understands method_exists on simple variables, not
// Assigning to a variable is the a workaround
// for false positive 'getCenterIDs doesn't exist errors suggested
// in https://github.com/phan/phan/issues/2628
Copy link
Contributor

Choose a reason for hiding this comment

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

Just adding this for documentation, #7309 was involved in this too

ridz1208 and others added 2 commits February 18, 2025 11:10
Co-authored-by: Saagar Arya <51128536+skarya22@users.noreply.github.com>
Co-authored-by: Saagar Arya <51128536+skarya22@users.noreply.github.com>
@skarya22 skarya22 self-requested a review February 18, 2025 18:29
Copy link
Contributor

@skarya22 skarya22 left a comment

Choose a reason for hiding this comment

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

LGTM! Tested with a custom script and all the filters work well.

@skarya22 skarya22 added the Passed manual tests PR has been successfully tested by at least one peer label Feb 18, 2025
Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

Most of these doc comments could have just been "{@inheritdoc}", but the ones written are fine too.

@driusan driusan merged commit 7cb0b59 into aces:main Feb 18, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Beginner Friendly PR or Issue appears to be easy for someone to use to familiarize themselves with LORIS Category: Feature PR or issue that aims to introduce a new feature Category: Refactor PR or issue that aims to improve the existing code Difficulty: Simple PR or issue that should be easy to implement, review, or test Language: PHP PR or issue that update PHP code Passed manual tests PR has been successfully tested by at least one peer Priority: High PR or issue should be prioritised over others for review and testing Project: C-BIG Issue or PR related to the C-BIG project Proposal PR or issue suggesting an improvement that can be accepted, rejected or altered
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants