Skip to content

Add policy author spec#3

Open
janedotx wants to merge 6 commits intodevfrom
add_policy_author_spec
Open

Add policy author spec#3
janedotx wants to merge 6 commits intodevfrom
add_policy_author_spec

Conversation

@janedotx
Copy link
Copy Markdown

@janedotx janedotx commented Apr 2, 2020

Explain pull request

This adds the Policy Author spec and removes the Geography related stuff in Policy, which is necessary now that Geographies will be covered in their own specs.

Is this a breaking change

None of the changes to Policy made here should affect any current implementations.

@janedotx janedotx requested review from avatarneil and marie-x April 2, 2020 03:22
Comment thread .gitignore Outdated
@@ -1 +1,2 @@
.venv/ No newline at end of file
.venv/
*.swp
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't be modifying the .gitignore

Comment thread policy-author/README.md Outdated
Parameters:
| Name | Type | R/O | Description |
| ------------ | --------- | --- | ---------------------------------------------- |
| `id` | UUID | Optional | If provided, returns one policy object with the matching UUID; default is to return all policy objects. |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems wrong, it should definitely be a policy_id parameter (which is what it is in the reference impl)

Comment thread policy-author/README.md Outdated

Payload: a new Policy object, without a `policy_id`

; a failure explanation on failure
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo?

Comment thread policy-author/README.md Outdated
- 401 - Unauthorized (if any auth issue)
- 500 - Server error (hopefully doesn’t happen)

### PUT /policies/{id}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should be /{policy_id}

Comment thread policy-author/README.md Outdated
- 409 - conflict (if immutable)
- 500 - server error

### POST /policies/{id}/publish
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/policies/{policy_id}/publish

Comment thread policy-author/README.md Outdated
- 500 - server error


### GET /policies/{id}/meta
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/policies/{policy_id}/meta

Comment thread policy-author/README.md Outdated
- 500 - server error


### PUT /policies/{id}/meta
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/policies/{policy_id}/meta

Comment thread policy/README.md Outdated

The machine-readable format allows Providers to obtain policies and compute compliance where it can be determined entirely by data obtained internally.

Policies will typically be linked to one or more associated geographies. Geography descriptions (e.g. geofences or lists of street segments) must also be maintained by the Agency indefinitely. Policies without specific geographies (global policies) are assumed to apply to the entire service area managed by the Agency.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't mention service areas, no longer a thing. Perhaps better to say something like "assumed to apply to all jurisdictions managed by the Agency".

Comment thread policy/README.md Outdated

Policies will typically be linked to one or more associated geographies. Geography descriptions (e.g. geofences or lists of street segments) must also be maintained by the Agency indefinitely. Policies without specific geographies (global policies) are assumed to apply to the entire service area managed by the Agency.

Geographical data will be stored as immutable GeoJSON, referenced by UUID. See the Geography and Geography Author specs for information on the Geography schema, and how Agencies are expected to create and maintain Geographies and serve them to Providers. In a future revision of Agency, we will deprecate the existing `/service_areas` endpoint. `/service_areas` currently only handles GeoJSON MultiPolygon and Polygon objects, and Policy documents might prefer Points for locations such as drop-zones. Policy may be used for a variety of enforcement actions, so it's important for the Agency to persist and keep immutable both Policy and Geography data.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GeoJSON FeatureCollection instead of just GeoJSON.

/service_areas has already been deprecated.

@avatarneil
Copy link
Copy Markdown

Sidebar to note -- even after these are approved you shouldn't merge to dev, we want to keep them as isolated branches to cut distinct PRs into OMF

@janedotx janedotx dismissed avatarneil’s stale review April 28, 2020 00:29

Changes implemented.

Comment thread policy-author/README.md
Response codes:

- 201 - Created. Returns: the Policy object on success, including a `policy_id` and a `version` indicating the current API version.
- 400 - Policy does not conform to schema
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can also send a 409 upon a conflict

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Which I suppose doesn't make sense considering we auto-fill a policy_id, I'll remove that error handling

Comment thread policy-author/README.md
- 500 - server error


### GET /policies/{policy_id}/meta
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should also have a 400 bad params error

Comment thread policy-author/README.md
- 200 - success
- 401 - unauthorized
- 404 - not found
- 409 - conflict (if already published)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should add 424 for a missing dependency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants