streetlives / yourpeer.nyc

The open source repository behind the yourpeer.nyc web application
https://yourpeer.nyc
MIT License
0 stars 0 forks source link

Membership data filter issue, possibly responding to hidden data? #107

Open adambard1 opened 7 months ago

adambard1 commented 7 months ago

I agree that the membership filter (checking the requirement to have Client of the organization) is not working as expected.

@jbeard4 I can see on GoGetta that if I select the checkbox to require Client of the Organization in the Personal Care Filter, locations appear that don't seem to have any membership requirement n the data on /team but I wonder if that is the only data the filter responds to?

Filter selection

Screenshot 2024-04-20 at 2 51 06 PM

The map definitely still shows locations that do not appear to have the Membership info filled on /team, so not requiring people to be clients of their organization, such as Catholic Worker's Maryhouse

Map shows

Screenshot 2024-04-20 at 2 50 31 PM Screenshot 2024-04-20 at 2 56 34 PM

I would expect it to be here: https://test.gogetta.nyc/team/location/3ada4162-ca1e-4d94-a178-e4e09aec0ab6/services/e6b553f5-00f8-4b46-8f5d-70adc0012956/who-does-it-serve

_Originally posted by @adambard1 in https://github.com/streetlives/streetlives_v2/issues/488#issuecomment-2067759932_

adambard1 commented 7 months ago

Re: "And this will still return Meatloaf Kitchen in the query results. Even though I don't think that Meatloaf Kitchen requires membership:

https://test.gogetta.nyc/find/Clothing/location/2e302015-6909-40de-85d4-9a84dc105d8c"

It provokes the question of whether the Membership filter is responding to seemingly missing data which might be in the db that would have been entered in the Who does it serve or membership field, because Meatloaf Kitchen's Kid's Corner Literacy service shows it serves Children, but I can't find that data anywhere in that service (as with Maryhouse above), and the membership field entry is None

Screenshot 2024-04-20 at 3 17 04 PM
jbeard4 commented 7 months ago

Well, here's the SQL query:

SELECT DISTINCT ("Location"."id")
    ,ST_DistanceSphere("position", ST_GeomFromGeoJSON('{"type":"Point","coordinates":["-73.98364047899548","40.72643155136306"]}')) AS "ASC"
FROM "locations" AS "Location"
LEFT JOIN "organizations" AS "Organization" ON "Location"."organization_id" = "Organization"."id"
INNER JOIN (
    "service_at_locations" AS "Services->ServiceAtLocation" INNER JOIN "services" AS "Services" ON "Services"."id" = "Services->ServiceAtLocation"."service_id"
    ) ON "Location"."id" = "Services->ServiceAtLocation"."location_id"
LEFT JOIN (
    "service_taxonomy" AS "Services->Taxonomies->ServiceTaxonomy" INNER JOIN "taxonomies" AS "Services->Taxonomies" ON "Services->Taxonomies"."id" = "Services->Taxonomies->ServiceTaxonomy"."taxonomy_id"
    ) ON "Services"."id" = "Services->Taxonomies->ServiceTaxonomy"."service_id"
LEFT JOIN "holiday_schedules" AS "Services->HolidaySchedules" ON "Services"."id" = "Services->HolidaySchedules"."service_id"
INNER JOIN "eligibility" AS "Services->Eligibilities" ON "Services"."id" = "Services->Eligibilities"."service_id"
INNER JOIN "eligibility_parameters" AS "Services->Eligibilities->EligibilityParameter" ON "Services->Eligibilities"."parameter_id" = "Services->Eligibilities->EligibilityParameter"."id"
WHERE (
        "Services->Taxonomies"."id" IN (
            '566956f6-00ad-4f75-ac68-3c56485ed489'
            ,'57727a6e-9bd1-4e1a-9c08-5be7d9f22b55'
            )
        AND "Services->HolidaySchedules"."occasion" = 'COVID19'
        AND ST_DistanceSphere("position", ST_GeomFromGeoJSON('{"type":"Point","coordinates":["-73.98364047899548","40.72643155136306"]}')) <= '5286'
        )
    AND (
        "Location"."hidden_from_search" = false
        OR "Location"."hidden_from_search" IS NULL
        )
GROUP BY "Location"."id"
    ,"Services"."id"
HAVING (
        (
            (
                (
                    CAST(json_object_agg("Services->Eligibilities->EligibilityParameter"."name", "Services->Eligibilities"."eligible_values") AS JSONB) -> 'membership' IS NULL
                    OR CAST(json_object_agg("Services->Eligibilities->EligibilityParameter"."name", "Services->Eligibilities"."eligible_values") AS JSONB) -> 'membership' ? 'true'
                    )
                )
            )
        )
ORDER BY "ASC" ASC LIMIT 1000;

The relevant part that concerns eligibility is here:

HAVING (
        (
            (
                (
                    CAST(json_object_agg("Services->Eligibilities->EligibilityParameter"."name", "Services->Eligibilities"."eligible_values") AS JSONB) -> 'membership' IS NULL
                    OR CAST(json_object_agg("Services->Eligibilities->EligibilityParameter"."name", "Services->Eligibilities"."eligible_values") AS JSONB) -> 'membership' ? 'true'
                    )
                )
            )
        )
jbeard4 commented 7 months ago

Here's an example of what the eligibility values look like:

{"gender": [], "membership": ["true", "false"]}

But I'm not sure what "membership": ["true", "false"] means here. @Rovack do you have any idea about this?

It looks like the the logic in the SQL query is saying:

  1. Include results without membership eligibility info (IS NULL)
  2. And include results that contain result true in the membership array.

But I'm not sure how to interpret these values in the membership array, so not sure if this check for true makes sense.

Thanks.

Rovack commented 6 months ago

Hey @jbeard4. If I'm not mistaken, the array under each eligiblity parameter is meant to represent all the valid values for that parameter, i.e. all the ones that are accepted by the service. So, in the case of membership, ["true", "false"] means that both people who are members, and people who aren't, are eligible to receive the service. ["true"] alone would suggest that only members are eligible, whereas ["false"] alone would theoretically mean that only non-members are allowed (which would be pretty weird).

In theory, it means that when filtering, it's usually enough to pass the desired value, and simply query for all services whose array contains the given value.

In the case of "membership required", though, it means that passing "false" would work as expected (you'd get back all services that accept non-members), but "true" would not - it would return all locations that accept members, whether or not they also accept non-members. If you actually wanted to find all services that require membership, the search would have to be for services whose eligibility doesn't include "false" (rather than ones that do include "true"), which is not something I believe the API currently supports.

Though admittedly I could never understand why "Required" was even an option for this filter, since even if you do have some membership, you'd still also be able to use services that don't require it. (Plus this filter always seemed oversimplified since it it only allows specifying "client of the organization", but not which organization you mean.)

And anyway, as for null, I believe the product requirement at the time was that a service should not be returned if it has no eligibility data (see this test), but should be returned in the case where it does have some eligibility data which just doesn't include a specific parameter (this case). The assumption is that this means the service doesn't have any restrictions regarding that particular parameter, and so will be returned regardless of what filter is applied for that param (as hinted by the name paramIsUnrestrictedOrAllowsValue). This is in contrast to the requirement for taxonomy-specific attributes (e.g. what type of clothes a service offers), where if no data is available for a given attribute, the assumption was that it should not be shown if its filter is used at all, regardless of the value (hence just attributeIncludesRequestedValue).

P.S. I see there's an open Asana task from 2019 about some of this complexity: https://app.asana.com/0/0/1128835683601538/f It doesn't cover the issue with how "Required" doesn't really filter anything out, but either way, I tend to agree with the 3rd option it proposes...

doobneek1 commented 3 months ago

@jbeard4 this is still an issue