apps-android-commons icon indicating copy to clipboard operation
apps-android-commons copied to clipboard

[Bug]: Category not found

Open Ainali opened this issue 2 years ago • 9 comments

Summary

I serached for a category that exists, but it doesn't show up.

Steps to reproduce

  1. When uploading an image, search for category: Amavenita

Expected behaviour

Category:Amavenita (ship, 2014) should be suggested.

Actual behaviour

"No Categories found"

Device name

Samsung Galaxy S10

Android version

Android 12

Commons app version

3.1.1~1c9267ca0

Device logs

No response

Screen-shots

Screenshot_20220318-180530_Commons

Would you like to work on the issue?

No response

Ainali avatar Mar 18 '22 17:03 Ainali

@Ainali isn't this the expected behaviour? Because, Amavenita (ship, 2014) is a non-hidden category and I think we are only supposed to show the hidden category in search bar. But I could be wrong.

devarsh-mavani-19 avatar Mar 19 '22 07:03 devarsh-mavani-19

Oh, I didn't realize. Then this is a duplicate with #4192 (and a use case for why it is needed, this is how all ships are categorized and the way for a human to identify them).

Ainali avatar Mar 19 '22 07:03 Ainali

"we are only supposed to show the hidden category in search bar"

@devarsh-mavani-19 I would consider that a bug. If a category is not hidden, then the upload wizard's category search step is supposed to show it.

nicolas-raoul avatar Mar 19 '22 09:03 nicolas-raoul

By the way, when I search for "rabbits", the first result is https://commons.wikimedia.org/wiki/Category:Rabbits which is not a hidden category:

adb

nicolas-raoul avatar Mar 19 '22 09:03 nicolas-raoul

By the way, when I search for "rabbits", the first result is https://commons.wikimedia.org/wiki/Category:Rabbits which is not a hidden category:

adb

Yes that is how it should work if it is non-hidden, then it should be displayed. However, in case of Ainali it was not displayed because category name contained a year which was older than previous year (Amavenita (ship, 2014)). so according to this issue #750 it should be filtered out.

so just to summarize currently 2 conditions must met 1. it should be non-hidden and 2nd either should not have any year in name or the year should be current or previous year.

devarsh-mavani-19 avatar Mar 19 '22 09:03 devarsh-mavani-19

However, in case of Ainali it was not displayed because category name contained a year which was older than previous year (Amavenita (ship, 2014)). so according to this issue #750 it should be filtered out.

Then I would still consider that issue to have been too poorly defined, because the app shouldn't filter out things where the year is part of the identifier of that thing, in this case, the ship that got the name in 2014. (Also, the app should probably not filter out by only older than current year, since it's common to upload files a day or two after they are taken which means that if I upload an image on January 1, I expect to see the category for the year before. Safer would be to filter categories where the year is less than current year minus 2.)

Ainali avatar Mar 19 '22 10:03 Ainali

I'm presuming the issue #750 was understood properly but the fix for it was a bit loose than it should've been. The relevant code snippet:

return item.matches(".*(19|20)\\d{2}.*".toRegex())
                && !item.contains(yearInString)
                && !item.contains(prevYearInString)
                || item.matches("(.*)needing(.*)".toRegex())
                || item.matches("(.*)taken on(.*)".toRegex())
                || item.matches(".*0s.*".toRegex())
                && !item.matches(".*(200|201)0s.*".toRegex())

That essentially returns true (which means the category is filtered out) when the name:

  1. Contains "19xx" OR "20xx" AND
  2. Does not contain current year AND
  3. Does not contain previous year OR
  4. Contains "needing" OR
  5. Contains "take on" OR
  6. Contains 0s AND
  7. Does not contain "2000s" OR "2010s"

If we were to convert the implicit precedence to explicit ones the condition looks like this:

(((1) AND (2) AND (3)) OR (4) OR (5) OR ((6) AND (7)))

Given short-circuiting evaluation, the condition should've returned true for "Amavenita (ship, 2014)" thus the category getting filtered out.

We could fix this by fixing the loose condition. I'll open a PR.

sivaraam avatar Mar 19 '22 10:03 sivaraam

Thanks a lot @devarsh-mavani-19 for finding this out! I had forgotten about this feature. Do we all agree on this?

  • Photographs taken on 2015-11-08 -> Hide
  • Amavenita (ship, 2014) -> Do not hide

nicolas-raoul avatar Mar 19 '22 13:03 nicolas-raoul

Do we all agree on this?

agree

devarsh-mavani-19 avatar Mar 19 '22 13:03 devarsh-mavani-19