Skip to content

Fix types of _id fields #525

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

Merged
merged 2 commits into from
May 3, 2025
Merged

Conversation

mdashlw
Copy link
Contributor

@mdashlw mdashlw commented Apr 27, 2025

Before you begin

  • I understand my contributions may be rejected for any reason
  • I understand my contributions are for the benefit of Derpibooru and/or the Philomena software
  • I understand my contributions are licensed under the GNU AGPLv3
  • I understand all of the above

I'm not sure why int and float parsers explicitly ignore spaces, didn't put that into the new parser

@mdashlw mdashlw force-pushed the id-search-field-type branch from 62e4fe6 to 5e04519 Compare April 27, 2025 10:42
@mdashlw mdashlw force-pushed the id-search-field-type branch from 5e04519 to 95f9f41 Compare April 27, 2025 12:30
@Meow
Copy link
Member

Meow commented May 2, 2025

As this change has something to do with indices, this is blocked similarly to #526 (see comments on that PR)

@mdashlw
Copy link
Contributor Author

mdashlw commented May 2, 2025

As this change has something to do with indices, this is blocked similarly to #526 (see comments on that PR)

This only touches query parsers not indices. Should be fine to merge

@Meow Meow merged commit 8d6d61e into philomena-dev:master May 3, 2025
6 checks passed
@mdashlw mdashlw deleted the id-search-field-type branch May 3, 2025 11:02
@@ -3,10 +3,10 @@ defmodule Philomena.Reports.Query do

defp fields do
[
int_fields: ~W(id image_id),
int_fields: ~W(id),
Copy link
Contributor

@MareStare MareStare May 4, 2025

Choose a reason for hiding this comment

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

@mdashlw why not turn id into a numeric_field too?

Copy link
Contributor

Choose a reason for hiding this comment

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

it needs to support range querying

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it doesn't make sense to support range queries for relational IDs like image_id? IIRC it should cost nothing, right?

@MareStare
Copy link
Contributor

MareStare commented May 4, 2025

QA

Comments

Galleries

  • image_ids:47 search for galleries works fine, although it seems to be exposed only in JSON API

Images

  • uploader_id:4 works for images

Posts

Reports

  • user_id:1 query works as expected
  • image_id:52 works as expected
  • reportable_id:240 works as expected too (generic for images, posts, and other types of objects)
  • admin_id:5 works as expected filtering only reports closed by the given user ID

@MareStare MareStare added the qa accepted The change was tested by another person (not the PR author) label May 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa accepted The change was tested by another person (not the PR author)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants