Avoid stripping additional characters from GIS SQL params. #1544
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1544 +/- ##
==========================================
+ Coverage 85.75% 85.91% +0.15%
==========================================
Files 36 36
Lines 1889 1889
Branches 313 313
==========================================
+ Hits 1620 1623 +3
+ Misses 190 187 -3
Partials 79 79
Continue to review full report at Codecov.
|
|
Ah, this needs to not run when it's not a GIS sql param. |
|
Now that I read the code again I'm really unsure. We're modifying the SQL which is being executed instead of only logging it and maybe fixing it later when trying to analyze/explain it. Is there a good reason for modifying the |
|
I'm not sure. Even though the tests indicate we test for postgis, it's not used. I'm working through setting it up locally to see if the tests pass. |
If the param lead or ended with characters identified in the strip calls, those characters would also be removed. Using split will retain all of the characters inbetween the given strings. When we drop support for < 3.9, we can switch to the removeprefix and removesuffix functions. Fixes #1543
Include mention in the docs about how to configure the user locally to install PostgreSQL extensions.
|
@matthiask I'm going to revert #1426. I don't think it ever covered the change and the test is broken. |
|
Replaced by #1547 |
If the param lead or ended with characters identified in the strip
calls, those characters would also be removed. Using split will
retain all of the characters in between the given strings.
When we drop support for < 3.9, we can switch to
the removeprefix and removesuffix functions.
Fixes #1543
This should add a test case that confirms the fix for 1543, but I'm unsure of what that would look like. If we think we absolutely need a test case for it, then perhaps we move it to
panels/sql/utils.pyso it can be in a unit test rather than an integration test.