piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

1103 Improve arrow function

Open dantownsend opened this issue 1 year ago • 3 comments

Resolves https://github.com/piccolo-orm/piccolo/issues/1103

dantownsend avatar Oct 18 '24 21:10 dantownsend

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 85.48387% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.42%. Comparing base (e2fa8f8) to head (87f8411). Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
piccolo/query/operators/json.py 84.84% 5 Missing :warning:
piccolo/querystring.py 60.00% 4 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1112      +/-   ##
==========================================
- Coverage   88.45%   88.42%   -0.04%     
==========================================
  Files         116      117       +1     
  Lines        8611     8653      +42     
==========================================
+ Hits         7617     7651      +34     
- Misses        994     1002       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Oct 18 '24 21:10 codecov-commenter

@dantownsend It seems that when we use the arrow function we need strings for queries (not booleans or other types). I think that makes sense because querystring uses strings. If we change the data type to string in these tests, everything passes.

def test_arrow_where(self):
    """
    Make sure the arrow function can be used within a WHERE clause.
    """
    RecordingStudio(
        name="Abbey Road", facilities='{"mixing_desk": true}'
    ).save().run_sync()

    self.assertEqual(
        RecordingStudio.count()
        .where(RecordingStudio.facilities.arrow("mixing_desk").eq("true")) # <-- here
        .run_sync(),
        1,
    )

    self.assertEqual(
        RecordingStudio.count()
        .where(RecordingStudio.facilities.arrow("mixing_desk").eq("false")) # <-- here
        .run_sync(),
        0,
    )

I also removed the load json test because the output(load_json=True) doesn't return a boolean after new changes.

I hope that makes sense.

sinisaos avatar Oct 19 '24 06:10 sinisaos

@sinisaos Thanks - you're right, the JSON values have to be strings.

dantownsend avatar Oct 19 '24 21:10 dantownsend

@sinisaos Thanks. I went a bit crazy on this in the end.

You can now go multiple levels deep, and traverse array elements within the JSON.

await RecordingStudio.select(
    RecordingStudio.facilities["a"]["b"][0]["c"]
)

dantownsend avatar Oct 23 '24 19:10 dantownsend

@dantownsend Wow! It's really amazing.

sinisaos avatar Oct 23 '24 20:10 sinisaos

@sinisaos Thanks. It turned into a bigger thing than I expected. But with this, we can potentially do better JSON filtering in Piccolo Admin.

dantownsend avatar Oct 23 '24 20:10 dantownsend