890774 - Add media + images to attribution

Status: VERIFIED FIXED

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Brett Gaylor [:brett]]

Currently the attribution "quote" is very underused and doesn't actually include all of the media from inside of a project.  Lets make sure images from image plugin, as well as all media clips, get included.

Comment 1

[Matthew Schranz [:mjschranz]]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/115

It's a start.

Mostly at this point what needs to be done is whatever UI considerations for this that we want. Can easily be taken by someone else for that.

Comment 2

[Matthew Schranz [:mjschranz]]

Comment on attachment 774715 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/115

These really should have been feedback. Also, not a large priority.

Comment 3

[Matthew Schranz [:mjschranz]]

Comment on attachment 774715 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/115

Too early to be editing tickets for me apparently.

Comment 4

[Matthew Schranz [:mjschranz]]

Comment on attachment 774715 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/115

This can actually be reviewed now.

Comment 5

[Matthew Schranz [:mjschranz]]

Couple of points.

1. This only shows sequencer clips that are actually used.
2. This doesn't show the base null media. Even if no clips are used. In my mind we don't need this in the attribution information as it isn't some external source we want to give credit to.
3. I only show flickr images for the images section. I have no idea how I would give attribution for random images.
4. I changed the old maps attribution to only actually show when maps are used. Seems pointless to include if it isn't.

Comment 6

[Scott [:thecount] Downe]

Comment on attachment 774715 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/115

This works well enough.

I have some thoughts, but really, I don't think any of them have to happen now. We can iterate after it lands.

I do have a few small comments in the pull request.

Not sure what we need Kate on here for. Can probably relieve her of this review for now.

Comment 7

[Scott [:thecount] Downe]

Comment on attachment 774715 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/115

This is fine, still on Kate for review, so I'll let Matt bug Kate about that, or decide if it is still needed at all.

Comment 8

[Matthew Schranz [:mjschranz]]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/140

I decided to redo this patch because I found it created too many weird issues for old projects before this patch.

Decided to do this all inside embed.js. The upside to this is there is basically only one minor UI annoyance for old projects that don't get republished. It shows the media data for the null media. Otherwise, this covers all problems for old projects from what I can tell.

Comment 9

[Scott [:thecount] Downe]

Comment on attachment 782705 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/140

Eh, I'm OK with the old media being in there. Kinda not important and we can iterate on that in the future.

I do however wonder how easy it would be to fix.

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/popcorn.webmaker.org

https://github.com/mozilla/popcorn.webmaker.org/commit/ff99dfb85726503b983a1e33d1bcba9fdbca31a5
Fix Bug 890774 - Add media clip data and images to embed attribution