iandees / wof-editor

A simple Who's on First editor.
https://writefield.nextzen.org/
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Update validation logic upstream in exportify for mz:is_current and other ints #4

Closed nvkelso closed 4 years ago

nvkelso commented 4 years ago

When we exportify we need to set some values to ints (not JS default strings).

See: https://github.com/whosonfirst/py-mapzen-whosonfirst-export/blob/master/mapzen/whosonfirst/export/__init__.py#L127-L140

nvkelso commented 4 years ago

See also:

nvkelso commented 4 years ago

Please also remember to update the VERSION (increment minor version).

stepps00 commented 4 years ago

Any of the -1,0, or 1 valued mz or wof property flags that should be encoded as ints should be added to this check. Eventually, the check should be added to the *-validate repo, but I'll adjust the exportify check to include relevant properties from:

stepps00 commented 4 years ago

Chatted IRL w @nvkelso, but the py-mapzen-whosonfirst-export repo may not be the best place to add property type checks. The existing exportify code only checks types for two property flags that should be in every record. The ask here is to check property types for properties that are not in all WOF records.

I can still update the exportify code, but it might be best to wire these checks in another way.

iandees commented 4 years ago

Since these are checks that should happen on all documents before they end up in WOF, not just when editing through the WOF Editor, I think it might be better to put these checks in the validator repo, not the export repo. But I'm ok with putting them in the WOF editor for now.

nvkelso commented 4 years ago

Validated only cries fire, doesn’t fix. So I think for now we fix these in WOF-editor app while general solution is arrived at.

Sent from my handsful device.

On Feb 5, 2020, at 06:02, Ian Dees notifications@github.com wrote:

 Since these are checks that should happen on all documents before they end up in WOF, not just when editing through the WOF Editor, I think it might be better to put these checks in the validator repo, not the export repo. But I'm ok with putting them in the WOF editor for now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

stepps00 commented 4 years ago

Here's a list of properties that should have integer values of either -1, 0, or 1. Eventually, these should each be checked in the validate tools, but it seems best for the wof-editor app to handle these check for now (rather than the exportify or validate code):

Optional properties, existing on some WOF records now, each should have int values of -1, 0, or 1:

Properties that are written to every record through the Exportify tool, each should have int values of -1, 0, or 1:

iandees commented 4 years ago

mz:is_current, mz:is_funky, and mz:hierarchy_label are the only items in that list the editor has fields for, and they will now (as of 7e3aa2b22ad5bf19791894ca0dfb701885ee6a5c) check that the value is an integer -1, 0, or 1.

nvkelso commented 4 years ago

Working for me, see https://github.com/whosonfirst-data/whosonfirst-data-admin-us/pull/40/files.

stepps00 commented 4 years ago

:tada: