Open paregorios opened 8 years ago
The connections in the json are there on the map; it's just that it's hard to see the ones that are not points because they display a shape rather than an icon, and it's small when zoomed out. I think it makes sense to use the representative point for connected places rather than the actual geometry, but that's a new feature request, not a regression, right?
A different problem is that the json doesn't include all of the connections listed on the page. I'm looking into that one now.
@davisagli should we make this ticket just about the missing stuff in the json, and create a separate story for the visualization issue?
Sure.
When logged in the only one missing from the json is http://pleiades.stoa.org/places/585903, which is expected because it's unlocated.
When logged out, http://pleiades.stoa.org/places/560221 is missing because its only precise location is in drafting, so it falls back to determining the repr point using connected places, and that fails on something with "No gridsquare c5 in map 60"
When logged out, http://pleiades.stoa.org/places/606653928 is missing because it's in the drafting state. This reveals two bugs with the new connections: 1. if a connection object is published that references a place that is not published, it is shown to anonymous users, and 2. in the same scenario, a logged-in user does not see any indication that the connection refers to a place in drafting.
I have created Scrumdo Story #66: use representative point for position of all connections on place maps for the visualization aspects of this ticket. I will next retitle this ticket to reflect the narrowing of its scope to missing items in the geojson.
Why http://pleiades.stoa.org/places/579885 is missing is a fun one. When adding a marker for a roughly located connected place, the connections-json view searches for getId=[id of connected place]. Well, the new connection object has the same id, so in some cases its finding the connection rather than the place, and the connection has no inherent geometry of its own. Will fix by also specifying portal_type='Place' in the query.
Regarding the first issue (of published connections referencing drafting locations), I believe we discussed this possibility a bit, but don't recall the outcome of that discussion. In the end though, it probably doesn't make sense to show users links to content they can't actually view.
Even with the fix I mentioned, http://pleiades.stoa.org/places/579885 is not showing up for anonymous users, and I'm stuck trying to figure out why. I'll come back to that one another time.
This is working much better (on staging) now that Alec implemented story 66 to show the representative point for connections, and with my fix from #143 so that wrong cap grids don't get in the way of locating a place by other means.
I think the only remaining thing we need to work on is what to do with connections where the Connection itself is published but the Place it refers to is not. Currently these are included in the connections list but do not show up on the map. I agree with Alec that we probably should not show these links to anonymous users. @paregorios do you agree?
@davisagli and @alecpm my inclination is to hide connections from unpublished places and to unpublished places from anonymous viewers. Authenticated viewers should be allowed to see them, even if they don't have permission to view the contents of the item (prevent duplicate creations). Make sense?
Yeah, that makes sense to me.
For the example Place above, I'm seeing two missing connections at this point. I think I understand why those specific ones are missing and perhaps we can decide how to handle similar cases going forward based on that.
1) Drymos: http://pleiades.stoa.org/places/585903 - has only a single location with no coordinates. The only point from which the geometry is derived is the one connection to Attica.
2) Daphni Monastery: http://pleiades.stoa.org/places/606653928 - has a location with coordinates, but that location is private. Because we are using the catalog metadata to derive representative point for connections, I believe that data point is not available (since the catalog metadata is not permission aware). This Place also has only one connection (to Attica), which is the only published item from which its location can be derived.
I'll need to do a little investigation as to why these aren't at least showing up on top of the representative point for Attica (perhaps there's an attempt to avoid recursion in the representative point calculation that leaves these out). In general though, the most efficient way to derive a representative point for each connection is to use the catalog metadata value for the place, which will only take into account published locations/connections.
So there are two concerns here:
I want to make sure we don't get these two concerns conflated.
I'll try to summarize again what I want to see in each case here in bit. Meantime, taking ownership of this ticket so I don't lose the todo in the shuffle.
still needs my attention
After the new-style connections roll-out, only those connected places with point geometries in
@@connections-json
show up on the place map:http://pleiades.stoa.org/places/579888
The user expectation is that there should be a connection symbol for each connection. The obvious solution seems to be to refactor
@@connections-json
so it uses the "representative point" for each connected place, rather than the bounding geometry.