Making WordPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#986 closed enhancement (fixed)

Display video producer credit on WPTV posts

Reported by: iandunn's profile iandunn Owned by: iandunn's profile iandunn
Milestone: Priority: normal
Component: WordPress.tv Keywords: good-first-bug has-patch
Cc:

Description (last modified by Otto42)

Per today's Community Team chat, people who contribute post production on WordPress.tv videos should be credited on the corresponding WPTV post.

At the bottom of the sidebar, we should add a new section titled "Video Producer", and then add a link to their WordPress.org profile in that section. The anchor text for the link would be their full name, rather than their username.

We already collect the producer's name on the Submit Video form, but we'll need to start collecting their WordPress.org username too.

We could have a note that we want the *individual's* name/profile, not their company.

They can leave the name/username blank, but if they enter a username we should make sure it's a valid WordPress.org account, and prevent the form submission if it isn't.

Since WPTV is hosted on WordPress.com, we'll need to submit some kind of request to WordPress.org to check the username. WordPress, BuddyPress, or api.wordpress.org might provide something. If not, we could add something to api.wordpress.org, or worst case just request their profile URL and analyize the HTTP response headers, because non-existant profiles get redirected to https://wordpress.org.

Attachments (4)

986.diff (28.7 KB) - added by BrashRebel 9 years ago.
Stab at giving credit to video producers.
986.2.diff (15.6 KB) - added by BrashRebel 9 years ago.
2nd patch, this time following @iandunn's instructions.
986.3.diff (9.1 KB) - added by iandunn 9 years ago.
Various minor tweaks to 986.2.diff
986.4.diff (13.4 KB) - added by BrashRebel 9 years ago.

Download all attachments as: .zip

Change History (33)

#1 @iandunn
9 years ago

  • Owner set to iandunn
  • Status changed from new to accepted

#2 @iandunn
9 years ago

s/We could have a note/We should have a note

#3 @iandunn
9 years ago

#613 would probably provide a way to validate usernames.

It'd be nice to backfill the existing posts with the producer usernames. If people are willing to collect all the data into a CSV, we could have a script handle the bulk update.

This ticket was mentioned in Slack in #outreach by iandunn. View the logs.


9 years ago

#5 @BrashRebel
9 years ago

  • Cc kyle@… added

#6 @BrashRebel
9 years ago

Since we'll be associating speakers with their .org profiles as well in the future (#987), should we add a field to the submit form asking for the speaker's .org user name as well? Currently we collect the names of both speakers and producers which become terms in their respective custom taxonomies. However, to link them to their .org profiles we'll need their user names in a meta field for both. Right?

#7 @iandunn
9 years ago

I think it'd be best to just collect the producer username for this ticket, but there's no reason we can't start collecting the speaker usernames now too, as part of #987.

Since #987 is such a big ticket, it'd be good to do it in small chunks; It'd also be good to start collecting the data now, so that we have more of it ready when the other pieces are in place.

So, there's nothing wrong with starting to collect both now, but that should be done in separate patches on the respective tickets.

#8 follow-up: @BrashRebel
9 years ago

With regard to the full name, are you thinking:

  1. We have an additional field for the Producer's name OR
  2. Add a name field to profiles.wordpress.org and grab that value

I guess if we do the latter in the future we can just amend this to use that but for now we should probably just get the name on WPTV, right? I guess I just hate asking for something if we can get it on our own.

#9 in reply to: ↑ 8 @iandunn
9 years ago

I guess I just hate asking for something if we can get it on our own.

That's definitely a good instinct, but not all producers will have WordPress.org accounts, so I think we'd need the name to fall back on.

We could require them to have an account in order to submit a video, but I don't think that was part of the original plan. I don't really have an opinion either way. @jerrysarcastic, what do you think?

#10 @JerrySarcastic
9 years ago

  • Cc jerry@… added

We could require them to have an account in order to submit a video, but I don't think that was part of the original plan.

I am fine with requiring a .org account for this, and having that be the only way to populate the "Video Producer" link on WordPress.tv

It encourages people to create an account, and gives us a place to display their submission history, achievements, and any other fun stuff we may want to look at adding there.

We could have a note that we want the *individual's* name/profile, not their company.

Requiring a .org profile I think does a silent job of enforcing this, but we can also add language to that effect to underscore that we want names of people, not organizations.

I leave it to individuals to find their own way to create a .org profile in a company name if they feel strongly about showing that in the video producer byline for videos on WordPress.tv. :)

#11 @BrashRebel
9 years ago

So here's what I'm thinking: add field for username (already done and submitting patch tomorrow if I have time). If we get both username and full name, use name as anchor text and link to w.org profile. If we get username but no full name, show username as anchor text. If we get full name but no username....we could use name as anchor text and link to the archive for that producer on wp.tv which lists all the videos they've produced?

#12 @iandunn
9 years ago

Replying to JerrySarcastic:

I am fine with requiring a .org account for this

Cool, then we could just remove the current Name field and add a Username field. It should be a separate field since in the past we stored names, and we don't want to mix names and usernames in the database.

Replying to BrashRebel:

So here's what I'm thinking...

That sounds good, because even if we require a username now, there are still older posts that will have a name instead.

I wonder if it'd be confusing to have it link to two different places, though? Especially since it probably won't be obvious to the user why or when a given destination will be chosen? Maybe just don't link to anything if they don't give us a username? I guess that also wouldn't be obvious why or when it changes from link to no link, so maybe that's no better?

Have you had a chance to look and see if there's a good way to verify if the username is valid? If not, I think it's ok leave that part out for v1, and just assume that it is.

#13 @BrashRebel
9 years ago

Checking the response headers from profiles.wordpress.org works just fine so I ran with it. Open to exploring a better way if you think necessary, but since it is only for the initial check when the form is submitted then it probably isn't a big deal which method we use.

As an option, we can adjust the wording of and around the link. Something like:

"Video produced by [Kyle Maurer](https://profiles.wordpress.org/brashrebel)"
and/or:
"View all videos by [Kyle Maurer](http://wordpress.tv/producer/brashrebel)"

or something along those lines. There's a difference between seeing the profile of the producer and seeing all the videos made by the producer.

So we're going with removing the old producer name field? So going forward we will only ask for the w.org username of the producer? Just making sure.

Last edited 9 years ago by BrashRebel (previous) (diff)

#14 @Otto42
9 years ago

  • Description modified (diff)

Checking the response headers from profiles.wordpress.org

Ick. We can do better than that.

Rather than hitting the profiles directly, have you looked at how we display gravatars here on places like Trac, which don't have access to the w.org user tables directly?

Compare the results of these two requests:

https://wordpress.org/grav-redirect.php?user=BrashRebel

https://wordpress.org/grav-redirect.php?user=non-existent-username

We may create an API for better user information in the future, but if you're going to do this as a hack for now anyway, I'd rather you hit an endpoint that a) isn't likely to change and b) is very low cost for us. Hitting profiles directly, not very low cost.

Just make sure you label the hack in the source so that it can be understood later.

Last edited 9 years ago by Otto42 (previous) (diff)

#15 @Otto42
9 years ago

  • Description modified (diff)

Whoops, didn't mean to change the description there.

Last edited 9 years ago by Otto42 (previous) (diff)

#16 @BrashRebel
9 years ago

Thanks for chiming in Otto. I'll modify the patch to use your recommended method.

#17 @iandunn
9 years ago

Replying to BrashRebel:

As an option, we can adjust the wording of and around the link

A format like "Video produced by Kyle Maurer" wouldn't be visually consistent with the rest of the items in that column, though. Maybe it'd be better to just let the link URLs change from post to post and not worry it. I don't really have a strong opinion. @JerrySarcastic, do you?


So going forward we will only ask for the w.org username of the producer?

Yup, that's correct. One side-effect of that is that we won't have their real name, just their username, since we don't have an API to get the real name from their username. I don't think that's a big deal, but if other do then we could just keep both fields.


Replying to Otto42:

Rather than hitting the profiles directly, have you looked at how we display gravatars here on places like Trac,

That's much better, thanks for pointing that out :)

This ticket was mentioned in Slack in #wptv by jerrysarcastic. View the logs.


9 years ago

This ticket was mentioned in Slack in #outreach by jerrysarcastic. View the logs.


9 years ago

@BrashRebel
9 years ago

Stab at giving credit to video producers.

#20 @BrashRebel
9 years ago

  • Keywords has-patch added; needs-patch removed

Just attached a new patch for this. First off, I'm not certain I'm generating my patch properly or not. I think I may have mixed something up when I created this branch and there are probably changes in this that don't pertain to this patch. I'm going to try and figure this out before my next attempt. That said, I think everything needed to address this ticket is here.

  • New field is added to video upload form
  • Username is checked when form is submitted. If it is valid, the video is submitted, otherwise the user gets an error.
  • New custom field for producer username is added to posts
  • Producer link is added to sidebar-single.php if it exists

Open to suggestions. If my patch is unusable I may ask for a hand figuring out how to generate them better. Sorry!

#21 @iandunn
9 years ago

The patch looks a lot closer to being compatible with SVN than previous ones, but there are still a few kinks to work out.

It looks like one problem is that it contains changes that were already committed to repository, so patch will try to apply them a second time, which fails.

You should be able to avoid that by calling git svn fetch on the master branch, and then merging those changes into your feature branch before you generate the diff.

Now that we've worked through a few of these issues, I took what we learned and creating a working draft of instructions for contributing with Git. Can you take a look and leave comments with anything that's inaccurate or incomplete?

Seeing the whole process laid out may also help identify any differences between the workflow you're using and the one that I think needs to be used.

#22 @BrashRebel
9 years ago

This is amazing @iandunn! Thanks for the excellent guide. I think I'll be able to manage nicely going forward. As soon as I get a chance I'll follow this guide and re-submit my patch for this ticket. Thanks again!

#23 @iandunn
9 years ago

Sounds good :)

@BrashRebel
9 years ago

2nd patch, this time following @iandunn's instructions.

#24 @BrashRebel
9 years ago

Added another attempt at the patch, this time following your excellent instructions.

#25 @iandunn
9 years ago

That worked perfectly :D

@iandunn
9 years ago

Various minor tweaks to 986.2.diff

#26 @iandunn
9 years ago

I made some minor tweaks in 986.3.diff, but overall this looks great :)

strpos() can return 0, which PHP will evaluate as false, so the strict comparison operator (===) was needed in dotorg_username_exists().

I removed calls to validate_username() and sanitize_user() because WordPress.org usernames are created with bbPress, which allows capitals (e.g., Ian Dunn Space Capital Test). They weren't really needed anyway, because the grav-redirect call will ensure that invalid usernames are thrown away.

The producer username should probably be taxonomy like the producer name was, rather than meta data, right? I think it makes sense to think of that as a taxonomy, it makes it more performant to query, and we get the archive page for free, in case we want to use that in future, like we discussed above.

I added a section to the Contributing With Git page about applying patches, can you test that out with 986.3.diff?

#27 @BrashRebel
9 years ago

Thanks for the help with that @iandunn and for the taxonomy suggestion. I've updated the patch to use a new taxonomy instead of a meta field. 986.4.diff now has everything I'm aware of that is needed. Anything else we need to do to this within the scope of this ticket?

@BrashRebel
9 years ago

#28 @iandunn
9 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 1724:

WordPress.tv: Give credit to video producers.

Fixes #986
Props BrashRebel

#29 @iandunn
9 years ago

Perfect, thanks Kyle :)

Note: See TracTickets for help on using tickets.