[WIP] - Fix location accessor #16
Conversation
|
@thet i've found two problems:
These two problems are related to p.a.vocabularies. The first one happens only with newer p.a.vocabulary and restapi because i think that something has changed in CatalogVocabulary. Are these changes ok for you? |
|
Another thing: i've made an upgrade-step to fix old fields, but it doesn't show up in products control panel (maybe because there are different profiles). From portal_setup everything works well |
|
@thet any news about this pr? |
There was a problem hiding this comment.
Great! Thanks for the PR. Although it seems we're loosing the default value for IVenue and IOrganizer schema and also we probably need some work to get location references from other subsites in a Lineage subsite working, I'm OK with this PR.
Sorry for the very late review!
| title=_(u'label_event_location', default=u'Location'), | ||
| description=_( | ||
| u'description_event_location', default=u'Select a location.' | ||
| ), | ||
| required=False, | ||
| missing_value='', | ||
| defaultFactory=default_location, | ||
| # defaultFactory=default_location, |
There was a problem hiding this comment.
why no default_location? the idea behind this was that some organizers have their events most always in the same location and with the overridable default location it can be pre-set.
| pattern_options={ | ||
| 'selectableTypes': ['Venue'], | ||
| 'basePath': get_base_path, | ||
| # 'basePath': get_base_path, |
There was a problem hiding this comment.
the idea behind the basePath was to allow access to venues outside the current plone site, e.g. if you're in a lineage subsite of another parent site and want to have access to the "address book" of that parent site.
but the catalog on the other hand looks much more convenient. you don't have to browse to the venues then, right?
| title=_(u'label_event_organizer', default=u'Organizer'), | ||
| description=_( | ||
| u'description_event_organizer', default=u'Select an organizer.' | ||
| ), | ||
| required=False, | ||
| missing_value='', | ||
| defaultFactory=default_organizer, | ||
| # defaultFactory=default_organizer, |
| vocabulary='plone.app.vocabularies.Catalog', | ||
| ) | ||
| form.widget( | ||
| 'organizer_uid', | ||
| 'organizer_ref', |
There was a problem hiding this comment.
if we use the vocabulary in the IVenue for the location_ref, shouldn'd we also use the vocabulary here?
| @@ -143,7 +143,15 @@ | |||
| handler="collective.venue.upgrades.upgrade_registry" | |||
| profile="collective.venue:base" | |||
| /> | |||
|
|
|||
|
|
|||
| <genericsetup:upgradeStep | |||
There was a problem hiding this comment.
bonus points for the upgrade step! (thanks ;) )
…ly serialized in plone.restapi
thet
force-pushed
the
fix_location_accessor
branch
from
68f4e16
to
37f9698
Compare
Location accessor for events didn't return the default "location" field value, if set.
Now the accessor looks for venue field, and if not set, looks into the default location field.
I've also added a test for this feature and setup travis conf.