[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

Improve presentation of GeoJSON+simplestyle JSON Schema validation failure
Closed, ResolvedPublic

Description

As part of T306533: Improve error handling and messages for maps and specifically T316213 we learned a lot about how the Maps (Kartographer) extension validates the GeoJSON content of an e.g. <mapframe>…</mapframe> tag. There are multiple different steps with very different semantics. Still all output is displayed via the same error reporting mechanism. The two validation steps relevant for this ticket are:

  • Is the JSON in the <mapframe> tag valid according to the GeoJSON specification?
  • Are the properties that are used in the GeoJSON valid simplestyle properties?

This is done via a single JSON Schema that combines GeoJSON+simplestyle and a generic validator, namely https://github.com/justinrainbow/json-schema. Because of the nature of GeoJSON+simplestyle, the nature of JSON Schemas in general, and the way our particular GeoJSON+simplestyle schema is written there are many possible ways how user-provided GeoJSON can be valid. When it's not, it's impossible (with this technology) to say which of the many possible interpretations is the closest one that requires the fewest fixes. The only thing the validator can do is to report the tree of possible interpretations and the points in this tree where it found a mismatch. This looks like this:

Screenshot from 2022-09-20 11-59-20.png (469×983 px, 110 KB)

The only thing we can do (with this technology) is to show the output of the validator to the user, ideally explain what it means, and let them figure out which of the suggested fixes is closest to what they intended.

Suggested improvements:

  • Make sure the validator output is not shown as a list of "errors" because that's not what it is.
  • Collapse it by default.
  • Make sure the one error message on top explains what the long list below means.
  • See if other validators create more useful output. This is already (partially) done. The output from https://github.com/swaggest/php-json-schema is very similar, even a bit more expressive, but fundamentally the same. Especially the same length.
  • See if it's possible to rewrite the schema so it creates more specific output. We tried but learned this is fundamentally how JSON Schemas work. Utilizing more recent JSON Schema features doesn't make a difference.
  • Make sure everything in the schema does have the best possible human-readable labels and descriptions.
  • See if the validator can show these instead of generic messages like "x does not match the regex pattern y". Partial result: Unfortunately neither the justinrainbow nor the swaggest lib make use of labels and descriptions.
  • …?

Event Timeline

Change 831867 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@master] [POC] Show JSON Schema validation log not as red error

https://gerrit.wikimedia.org/r/831867

Change 830254 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@master] Much simpler messages for most basic GeoJSON validation errors

https://gerrit.wikimedia.org/r/830254

Change 832537 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@master] [WIP] Improve titles & descriptions in the GeoJSON Schema

https://gerrit.wikimedia.org/r/832537

awight renamed this task from Improve presentation of GeoJSON+simplestyle JSON Schema validation failure logs to Improve presentation of GeoJSON+simplestyle JSON Schema validation failure.Sep 16 2022, 8:51 AM

Change 830254 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Much simpler messages for most basic GeoJSON validation errors

https://gerrit.wikimedia.org/r/830254

@ECohen_WMDE, this is the presentation currently proposed in https://gerrit.wikimedia.org/r/831867:

Screenshot from 2022-09-20 11-59-20.png (469×983 px, 110 KB)

  • Collapsed by default. The collapse button is standard behavior of MediaWiki, but i think we can change how it looks, if needed.
  • Same color and font as <code> elements, because that's more or less what it is: console output.
  • The red part on top is unchanged.

I suggest to merge this first patch as it is and possibly tweak details (font, line-height, margins, colors, …) later, if needed. The idea here is not to create pressure but to release pressure and give us more time to think about this, while we can already get rid of the large red box.

As discussed in the story time, I am fine with the changes listed above. Go ahead and merge, since they are definitely better than the large red box.

If working on the error messages is prioritized, then we can talk about what else to improve later.

Change 831867 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Show JSON Schema validation log not as part of red error list

https://gerrit.wikimedia.org/r/831867

thiemowmde claimed this task.

The Documentation patch https://gerrit.wikimedia.org/r/832537 is not done, but should get a separate ticket if we continue working on it. This is good enough for now, considering the limited resources we wanted to spend on this.

Change 839463 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Kartographer@master] Fix error reporting mistaken array of GeoJSON as array of errors

https://gerrit.wikimedia.org/r/839463

Change 839463 merged by jenkins-bot:

[mediawiki/extensions/Kartographer@master] Fix error reporting mistaken array of GeoJSON as array of errors

https://gerrit.wikimedia.org/r/839463