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

Fix the filter fails when property name contain apostrophe characters #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pastakhov
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

Codecov Report

Merging #54 (9be90c0) into master (c517061) will not change coverage.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##             master      #54   +/-   ##
=========================================
  Coverage     68.98%   68.98%           
  Complexity      740      740           
=========================================
  Files            36       36           
  Lines          2086     2086           
=========================================
  Hits           1439     1439           
  Misses          647      647           
Impacted Files Coverage Δ
includes/Specials/BrowseData/SpecialBrowseData.php 84.33% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@krabina
Copy link
Contributor

krabina commented Mar 24, 2023

@gesinn-it-gea can you have a look, please?

@gesinn-it-gea
Copy link
Member

@pastakhov many thanks for your PR! We are busy with other things at the moment. It might take some time before we can work on SD.

@gesinn-it-wam this PR needs to be carefully reviewed. At first glance, it looks like escaping was intentionally implemented in the original code like this. Apostroph usually causes issues when passed to SQL. This might have been the reason 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.

4 participants