Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API change: related Events saved in Metadata changes from JSONArray to JSONObject #297

Closed
stefanmedack opened this issue Jan 4, 2018 · 7 comments

Comments

@stefanmedack
Copy link
Contributor

This API change is not very recent, but I always forgot to add an issue for it. For some reason the field of related events saved in the Metadata of an Event has been changed from JSONArray to a JSONObject in the API. This leads to problems in parsing the Metadata for example in https://github.com/johnjohndoe/c3media-base by @johnjohndoe or in my project that uses this library https://github.com/stefanmedack/cccTV

Is it possible to revert this change at least for the API? I would also argue that it does not make sense to save a list of related events as a key/value pair, since it is a list and can now hardly be parsed or iterated (at least in Java/Kotlin, but most likely in other languages as well).

@saerdnaer
Copy link
Member

Can you provide before/after example snippets?

@manno
Copy link
Member

manno commented Jan 8, 2018

@stefanmedack or just the API URL where this happens

curl -v -H "CONTENT-TYPE: application/json" 'https://media.ccc.de/public/events/search?q=ccc&page=2' | jq '.events[] | .metadata'

gives a list of these:

{
  "related": {
    "2983": 2,
    "3466": 2,
    "3493": 2
  },
  "remote_id": "7726"
}

That change was required i.e. to prune weak related entries periodically: https://github.com/voc/voctoweb/blob/master/lib/update_related_events.rb#L16

I'm sorry if that broke your application, but it is a required change. Can't you just call {}.keys() or something?

@stefanmedack
Copy link
Contributor Author

Okay, in this case that is unfortunate for us. But again: wouldn't it make sense to still use a list of objects. So I would suggest to change the structure to

{
  "related": [
     {
         "eventId" : "2983": 
         "whateverThisValueIsFor": 2
     },
     {
         "eventId" : "3466": 
         "whateverThisValueIsFor": 2
     }
  ],
  "remote_id": "7726"
}

@manno
Copy link
Member

manno commented Jan 15, 2018

I don't think it would make sense to change that for voctoweb internally?
By using a hash data structure (id: weight) voctoweb gets unique keys and merging based on keys for free.

I would consider a PR which change this externally for API clients in the view at app/views/public/shared/_event.json.jbuilder.

@johnjohndoe
Copy link
Contributor

johnjohndoe commented Feb 11, 2018

@manno Do I correctly understand that you would be fine with changing the public API for public/events/ to output the following:

...
{
  "related": [
     {
         "event_id" : "2983",
         "weight": 2
     },
     {
         "event_id" : "3466",
         "weight": 2
     }
  ],
  "remote_id": "7726"
}

It would be great to get help with how to modify the builder with regards to the related property. It is not mentioned in the json builder. Are you using the master branch for production?

@saerdnaer
Copy link
Member

saerdnaer commented Feb 11, 2018

Idea for the future: We should thinking about solving such issues with a new GraphQL based API endpoint.

@johnjohndoe
Copy link
Contributor

✅ Fixed in the c3media-base client library here. /cc @stefanmedack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants