Page MenuHomePhabricator

Invalid coordinates cause table row to disappear
Closed, ResolvedPublic

Description

Reproduce: declare a cargo table with a Coordinate Field and a String field holding e.g. the name of the place, pass a string to the coordinate field in {{#cargo_store}}; For example {{#geocode: this place does not exist}} from the Maps extension returns Geocoding failed; the row is not saved

The function parseCoordinatesString in includes/CargoUtils.php (line 977) throws a MWexception if the coordinates in a Cargo Coordinates Field are not in a valid format; I cautiously assume this breaks the Cargo store flow; Because a) the deleted page does not get rewritten; b) it passes the blankOrRejectBadData() method in /includes/parserfunctions/CargoStore.php

Would it be feasible to just return null for the coordinate field? Why bothering about invalid coordinates at this stage? Shouldnt that be up to libraries/code that actually processes these coordinates for display, etc?

From what I see couldn't it be set to null in blankOrRejectBadData()?
Or by setting it to null in the two functions coordinatePartToNumber and parseCoordinatesString and then handle it in the 2 calls in CargoStore.php on line 385 and below

Quick example: I display loactions in a table and on a map; While it does not matter if the point is shown on the map, the location should at least be shown in the table. I would need to store the same data in two different cargo tables...

fix/workaround: convert {{#geocode}} coordinates string to single number and check if its a number with {{#expr}} and then conditionally save to {{#cargo_store}} with {{#iferror: expr||validCoordinates}}; still happy to provide a PR if you think it makes sense (although I am not at all familiar with Cargo code base)

Event Timeline

Sorry about the problem. I just checked in what I think is a fix for this - if you get the latest code, please let me know if it works better now.

No need to be sorry. Thank you for the blitz fix! It works great!

Yaron_Koren claimed this task.