lacuna-tech / mds-core

Repo for LADOT MDS implementation for contribution to the Open Mobility Foundation
Apache License 2.0
13 stars 9 forks source link

Add modalities filter option to readPolicies #1052

Closed BigIdeas closed 2 years ago

BigIdeas commented 2 years ago

📚 Purpose

FHV conductor requires the ability to filter policies by modalities. This adds support for that to mds-policy-service. A future PR will bring this new filter param into mds-gql.

Any policy with at least one rule matching any of the requested modalities will be returned.

While working on this I noticed that in city-dev there are policies without rules, policies whose rules are an empty array, and policies with rules that have no modality. I'm not sure whether we should expect policies like those in production, but I wrote the modality query such that those will all be returned by a query for micromobility (the default modality) just in case.

🗄️ Database Changes:

💔 Breaking Changes:

changeset-bot[bot] commented 2 years ago

🦋 Changeset detected

Latest commit: 988580c2db2dd887147b0bd9a4863f4f84f42807

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 28 packages | Name | Type | | ------------------------------------------ | ----- | | @mds-core/mds-policy-service | Minor | | @mds-core/mds-compliance-api | Patch | | @mds-core/mds-compliance-batch-processor | Patch | | @mds-core/mds-compliance-engine | Patch | | @mds-core/mds-compliance-service | Patch | | @mds-core/mds-db | Patch | | @mds-core/mds-policy-author-api | Patch | | @mds-core/mds-policy | Patch | | @container-images/mds-policy-service | Patch | | @container-images/mds-compliance-api | Patch | | @container-images/mds-compliance-service | Patch | | @mds-core/mds-agency | Patch | | @mds-core/mds-api-helpers | Patch | | @mds-core/mds-audit-api | Patch | | @container-images/mds-policy-author-api | Patch | | @container-images/mds-policy | Patch | | @container-images/mds-agency | Patch | | @mds-core/mds-collector-api | Patch | | @mds-core/mds-geography-api | Patch | | @mds-core/mds-geography-author-api | Patch | | @mds-core/mds-jurisdiction-api | Patch | | @mds-core/mds-transaction-api | Patch | | @container-images/mds-audit-api | Patch | | @container-images/mds-collector-api | Patch | | @container-images/mds-geography-api | Patch | | @container-images/mds-geography-author-api | Patch | | @container-images/mds-jurisdiction-api | Patch | | @container-images/mds-transaction-api | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

janedotx commented 2 years ago

@BigIdeas We're considering having policies with no rules be deactivating policies, and if we go through with that, they shouldn't ever show up in the front-end.

BigIdeas commented 2 years ago

Does this need a corresponding AJV query param validation update too?

Good question but... I don't think so? I'm not seeing any existing validation of the readPolicies query params. Very happy to write a quick validator if we're moving in that direction though.

nessur commented 2 years ago

Does this need a corresponding AJV query param validation update too?

Good question but... I don't think so? I'm not seeing any existing validation of the readPolicies query params. Very happy to write a quick validator if we're moving in that direction though.

@avatarneil yeah, checked, none of the queries on Policy Service validate their input. In mds-policy-api there's a bunch of parsers, but they're express-style parsers, not AJV-ish things.

BigIdeas commented 2 years ago

@BigIdeas We're considering having policies with no rules be deactivating policies, and if we go through with that, they shouldn't ever show up in the front-end.

Awesome, good to know! I'm thinking I'll keep this PR as is now - err on the side of showing all policies - and can easily delete the extra query for null rules if/when we start using those to deactivate policies.