maiera / gde-app

Apache License 2.0
22 stars 9 forks source link

Automatically update youtube video views into metadata.impact #201

Closed patt0 closed 9 years ago

patt0 commented 9 years ago

This is to run during the weekly ActivityPost impact update

patt0 commented 9 years ago

@SmokyBob @Scarygami

Gents I have done this as part of the weekly cron job and run it on elites datastore.

Before I push the PR I want to verify that this has not broken anything for the Googlers Login or for anyone that does have some video's for which the metadata was updated.

Also how would we want to deal with this? ActivityRecord: id=4589383814479872

During implementation of this feature https://github.com/patt0/gde-app/commit/2255cd71636c5d76b02a2dc34a15c5648cdcc21f its clear that in many cases the metadata list is empty and needs to be created. At this stage what are the minimum information of important we can add to it beyond type, link and group ( which i have not done yet ) ?

Further you will see that I have a naive implementation, assuming there is only one metadata record per ActivityRecord ... is it and will it ever be otherwise?

Scarygami commented 9 years ago

Assuming only one metadata record is too naive I would say, I have some where I have more than one, and you might overwrite manual impact data from a wrong metadata record.

I can imagine combinations of video with opensourcecode, techtalk with video, ...

For the activity to display correctly in the frontend we should also make sure to add #video to the activity types of the AR if it is not there already.

Then check the existing metadata records for one with the same youtube link and only update the impact on this one, or create a new metadata record otherwise, not picking one of the non-matching ones.

So I would suggest this approach:

  1. Check if the activity link is a youtube link -> If yes
  2. a) check metadata for existing link, and create a new one if necessary.
  3. b) add #video to activity types if it isn't there yet.
  4. For all AR find youtube metadata and update the impact. This will also catch cases where the youtube link isn't the main activity link but has been added manually as metadata. (For example if I add a recording to an event.

@SmokyBob can you check what happens in the frontend when I have several activity types that translate to the same activity group? At the moment it seems that if I have an activity with e.g. #article and #video I only get one #content metadata but would need two separate ones for this.

SmokyBob commented 9 years ago

@Scarygami yes metadata are one for each AG associated to the selected ATs, maybe a video AG? I think a separated AG for video would make sense because make sense to have a blogpost with a video or a written tutorial with step by step video as the same activity, and 2 different "impacts" to track. What do you guys think?

@patt0 about ActivityRecord: id=4589383814479872 I think we should skip it if there's no "quick" way to have the playlist statistic.

+1 for @Scarygami proposition about splitting the find youtube link and update #video impact.

P.S. added a comment on the code to get the video ID P.P.S. It's good to finally be able to fully understand what the code does on the backend :)

patt0 commented 9 years ago

Gents have a look at this commit https://github.com/patt0/gde-app/commit/f0512f47eca2b8cfc6eb2a78e460de0d7816b857 its pretty much what we want. I am going to run a test on the production data, by copying it to elite.

SmokyBob commented 9 years ago

Added a comment in relation to your finding of the strange activities link. Everything else looks perfect.

Scarygami commented 9 years ago

One thing I noticed: the auto-generated metadata set is not visible in the frontend if there is no #video tag in the activity types. I guess we should adapt the frontend to show all metadata from the datastore-record even if it doesn't match the activity types.

SmokyBob commented 9 years ago

I think we should add the #video tag to the activity types if an activity_metadata for the video is added, too keep things consistent

Scarygami commented 9 years ago

There would still be a small issue since the current script might in same cases create a second video metadata object, and the frontend would only display one of them. I have some ideas how we can solve this and other problems once more different data sources come in. We can talk about it in the next app hangout, which I will hopefully be able to join.

SmokyBob commented 9 years ago

:+1: As the App hangouts are now monthly, if we think we need a "dev focused" hangout we can set up one to discuss issues/ solution in a more detailed way. I'll try to catch up with the SO integration topic today and we could schedule a Dev hangout for next week

patt0 commented 9 years ago

+1

Patrick Martinent

On 26 March 2015 at 19:46, Mauro Solcia notifications@github.com wrote:

[image: :+1:] As the App hangouts are now monthly, if we think we need a "dev focused" hangout we can set up one to discuss issues/ solution in a more detailed way. I'll try to catch up with the SO integration topic today and we could schedule a Dev hangout for next week

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/201#issuecomment-86534450.