Page MenuHomePhabricator

Add support for categories on the frontend
Closed, ResolvedPublic3 Estimated Story Points

Description

Frontend for supporting categories as inputs from users. Tasks for this ticket:

  • Add an accordion section for "Categories" under the current "Participants" accordion.
  • Below the heading is some help text for the section: Event data would be generated by counting participant edits in the specified categories. If no participants are specified, all edits to the category during the event will be counted.
  • Add a blank text input for category name and a dropdown for wiki next to it, by default.
  • The 'wiki' dropdown contains the wikis the user picked for the event. In the case of *.wikipedia, it would contain all wikipedias and so on. The wikis appear in the list by their full name - English wikipedia, German wikisource etc.
  • Add a button to allow user to add more inputs as above. This behavior is similar to how adding Organizers works. When the βž• button is clicked, a blank text input is added for the category and a dropdown wiki input is added with the same wiki as the user last picked.

Tentative mock:

image.png (656Γ—1 px, 209 KB)

Note that subcategories in the mock will be added in T200481: Allow for subcategories to be included when parsing categories.

Event Timeline

Niharika triaged this task as Medium priority.May 15 2018, 12:30 AM
Niharika created this task.
β€’ Vvjjkkii renamed this task from Add support for categories on the frontend to wycaaaaaaa.Jul 1 2018, 1:10 AM
β€’ Vvjjkkii raised the priority of this task from Medium to High.
β€’ Vvjjkkii updated the task description. (Show Details)
β€’ Vvjjkkii removed a subscriber: Aklapper.
CommunityTechBot renamed this task from wycaaaaaaa to Add support for categories on the frontend.Jul 2 2018, 5:24 AM
CommunityTechBot lowered the priority of this task from High to Medium.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added a subscriber: Aklapper.
Niharika set the point value for this task to 3.Jul 25 2018, 11:36 PM
Niharika moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.

From T201710 arose the concern that with our current setup, the category form will be separate than the participant form. This means you can't add participants and categories at the same time.

I assume this is an issue? It's a little clear because each section has it's own submit button, as opposed to there being one at the bottom.

I can rework it to be a single form but we might consider adding another submit button so it's clearer how it works.

Or... we could get fancy and submit via AJAX. This actually shouldn't be too hard, because we have a Twig view for each form. The JavaScript submits the forms, gets back the HTML and replaces that part of the DOM. This would offer a more fluid experience, too.

From T201710 arose the concern that with our current setup, the category form will be separate than the participant form. This means you can't add participants and categories at the same time.

I assume this is an issue? It's a little clear because each section has it's own submit button, as opposed to there being one at the bottom.

I can rework it to be a single form but we might consider adding another submit button so it's clearer how it works.

Or... we could get fancy and submit via AJAX. This actually shouldn't be too hard, because we have a Twig view for each form. The JavaScript submits the forms, gets back the HTML and replaces that part of the DOM. This would offer a more fluid experience, too.

I think it's okay for them to be different forms. It seems clear in the UI because -

  1. Both forms have separate submit buttons with different labeling (Save participants and Save categories).
  2. The forms live in their own separate accordion sections.

Let's roll with separate forms and we'll improve the UI if we see this as a problem.

MusikAnimal moved this task from Backlog to In progress on the Grant-Metrics board.
MusikAnimal moved this task from Ready to In Development on the Community-Tech-Sprint board.

Works for me! Thanks

Let's roll with separate forms and we'll improve the UI if we see this as a problem.

Just so you know, it will be a significant amount of work to later change this to be one form instead of two separate forms. So if there's any doubt, let's try to decide on something now :) I am certainly okay with separate forms, and I think that makes the most sense.

@MusikAnimal Let's do separate forms only. That's why I said we'll improve the UI if it becomes a problem.. My feeling is that if in future we end up adding lots more to the form (we might with Event tools that Joe is working on), it will get gigantic and we'll have to look into breaking it down. I'd rather leave it separate and focus on making the UI better so it's very clear that they are separate forms.

@Prtksxna Flagging this for your opinion.

Well I was talking to Sam, and I think we might try to step away from Symfony Forms. That would make this easier, and also free us up from the technical limitations discussed at T201710: Clarify when usernames in Participants list are saved or unsaved. We don't need to rebuild those forms yet, but we can build any new forms with our system. We need to do some experimentation first, though. More info to follow!

Let me update with some of the challenges I'm facing. I'm still going back and forth with Symfony Forms. My pain point was building the view using FormBuilder, which basically is building an HTML form using PHP. That's awfully weird, and it's clear we don't want to do that anymore. But, it seems Forms are still used as "factories", such that you construct what the POST body would consist of, and giving it a big hash of nested key/value pairs will build the model object for you and all the relationships. That much we do want, otherwise we have to build it from scratch. So... I'll be yet again giving Forms another try, this time using it solely as a factory-building thingamabob, and not using FormBuilder to construct what will be shown in the view. Not sure if that makes sense. I found this guide to be helpful, although it is from 2012: https://williamdurand.fr/2012/08/02/rest-apis-with-symfony2-the-right-way/


That aside, there's a few UI-related issues I should bring up:

The 'wiki' dropdown contains the wikis the user picked for the event. In the case of *.wikipedia, it would contain all wikipedias and so on.

Once other wiki families are the mix (Wiktionary, Wikibooks, etc.) this list would be too big for a dropdown. So I'm trying to use an input field, where the autocompletion only pulls from wikis configured on the event. We of course need to validate that the wiki entered is a wiki configured on the Event, but we'd want to do that anyway and not 100% rely on the frontend for validations.

The wikis appear in the list by their full name - English wikipedia, German wikisource etc.

This is not trivial to do. We're showing the domain everywhere else, so let's maybe create a separate task and discuss further. I am under the impression people understand what domains are, at least the users of Grant Metrics will. I often point to XTools and Pageviews Analysis as examples for frontend matters, since they receive a significant amount of traffic. Both tools only accept domain names (though XTools also accepts database names), and I haven't gotten any complaints.


Some other related work:

  • Translating category IDs to category names (since we only store IDs, as we should). This isn't that bad. We have a property on the EventCategory class, title, that isn't mapped to the database. We use what's called an "event subscriber" so that whenever an EventCategory is loaded, it automagically puts in the title, and similarly it will put in the category ID from the title when you go to persist it to the database. We do the same thing for Organizers (user ID and username) with UserSubscriber. This class once did the same thing for Participants, but since there could be hundreds of participants shown on a page, we batch-fetch the usernames for efficiency. There won't be but a handful of categories on the event page so I think it's OK to do single queries, for the sake of cleaner code.
  • Resolving some huge complexity around "wiki families". This is hard to explain, but here it goes:
    • For a background on the different types of EventWikis, see T189915#4164685
    • I originally planned on each type being a different model (OrhpanEventWiki, FamilyEventWiki, etc.), but settled on doing the logic with various methods (EventWiki::isFamilyWiki(), etc.). Maybe that was wrong, but it's too late to change this I think.
    • *.wikipedia (or *.wiktionary, etc.) is a single EventWiki object (where isFamilyWiki() would be true), that belongs to an Event. When statistics are generated, an individual EventWiki is created for each child wiki (e.g. fr.wikipedia) for which there are statistics. This is how we know what wikis to show on the event page (T192579). That all makes sense.
    • The issue now is validating wiki input for each category. We need to make sure it belongs to one of the wiki families already configured on the Event. Say I type in fr.wikipedia, when there is no EventWiki for this yet, but there is an *.wikipedia EventWiki. A new EventWiki needs to be created for fr.wikipedia, even though there may be no stats for it. Seems simple but it's really complicated... anyway I've got a plan and it's in the works.
  • Making multiple forms that submit to the same controller action. We talked about this in earlier comments of the task. We definitely want different forms (Save Participants, Save Categories, more in the future). The tricky part is that they all submit to EventController::showAction() (necessary because we can't pass around Event objects from action to action), so in that method we need ensure we only handle the form that was submitted. I've already figured this out, so all good, but it wasn't easy!

There are numerous other things I've done as part of category support, but the above summarizes the most difficult parts. Hopefully that clears up why this is taking so long. It is better to get this done correctly from an infrastructure standpoint, so we don't end up redoing things later. Once this is all over with I think it will be easier to add more forms to the event page (which we will most likely, for template transclusions, and for entering individual pages).

Thanks for the patience!

Thanks for this level of detail @MusikAnimal. This kind of context is great to have so that everyone on the team can understand and potentially help.

I have a dumb question: where does this category tool live within Grant Metrics? E.g., is it on the Event setup page? If I understand this feature, it is basically a filter to limit results to certain categories. So the user will get results that match the available combination of a) the specified timeframe, b) the specified users if applicable, c) the specified categories. Is that right? Is there a design to show how it will fit on the page?

I have a dumb question: where does this category tool live within Grant Metrics? E.g., is it on the Event setup page? If I understand this feature, it is basically a filter to limit results to certain categories. So the user will get results that match the available combination of a) the specified timeframe, b) the specified users if applicable, c) the specified categories. Is that right? Is there a design to show how it will fit on the page?

Yes, that's correct. It goes underneath the Participants section on the event page. There's some more mocks on the project page that Danny made a while back so they're slight outdated.
If you go to this event it is easy to see that there is a participants section. The category section will appear below it. They are collapsible sections.

While looking at this ticket yesterday it occurred to me that we had not mocked up or discussed what the categories would look like when they are pulled from the database for an event which already has categories or what the behavior would be when the user clicks Save categories. Completely my fault for overlooking that.

We could very well just populate the input boxes the same way we input them or we could do something like we do for participants.
Since this ticket has been large enough already, I have split out that work into T202762: Behavior on category save/pre-population and asked @Prtksxna to take a look at it.

@aezell @Mooeypoo @Samwilson An update: I finally got Symfony Forms (proper) to work! And I feel pretty stupid for not having realized what I was doing wrong. Once it clicks mentally, Symfony Forms aren't so hard, after all, and they make a lot of sense. Now I get why it's a convention!

To make things even more simple, I did away with the EventCategory -> EventWiki -> Event relationship. Instead, Event has many EventCategories, simple as that, and we just store the wiki domain instead of using a foreign key to EventWiki. EventWiki doesn't give us any benefits in this case, so I think this is fine.

Here are some preliminary PRs to make the actual frontend one more reviewer-friendly:

More PRs to come!

@MusikAnimal I read through these changes briefly and it seems much more straightforward on the surface. I'll spend some more time with it later today.

A rough demo is now on https://tools.wmflabs.org/grantmetrics-test. Feel free to give it a try!

There a few other minor things left to do, but what's there should at least be working. PR is at https://github.com/wikimedia/grantmetrics/pull/115 though it isn't ready for a full review. I'm going to break it out into a few separate PRs, too.

Notes:

  • Wiki input comes first, conflicting with the mock. This is because we need that to be filled out in order to have autocompletion on the category title.
  • If there is only one wiki configured on the event, that wiki is automatically put in the category form and the wiki input is disabled.
  • Now that there are two forms on the event page, errors in the forms may be harder to spot. To help with this I added some JavaScript that will scroll the window to the form that has errors.

Known outstanding issues:

  • When categories form is in invalid state, the titles are underscored. Still not sure how to fix that, but I will give it another try.
  • You tell me... :)

A rough demo is now on https://tools.wmflabs.org/grantmetrics-test. Feel free to give it a try!

This is awesome!!

  • Wiki input comes first, conflicting with the mock. This is because we need that to be filled out in order to have autocompletion on the category title.

Sounds fine.

  • If there is only one wiki configured on the event, that wiki is automatically put in the category form and the wiki input is disabled.

Looks good to me.

  • Now that there are two forms on the event page, errors in the forms may be harder to spot. To help with this I added some JavaScript that will scroll the window to the form that has errors.

That's cool!

Known outstanding issues:

  • When categories form is in invalid state, the titles are underscored. Still not sure how to fix that, but I will give it another try.
  • You tell me... :)

I found a few more issues. Would you prefer I create bug tickets for those?

I found a few more issues. Would you prefer I create bug tickets for those?

This isn't deployed or merged yet so here is fine for now. Thanks!

Okay.

  • From task description:

When the βž• button is clicked, a blank text input is added for the category and a dropdown wiki input is added with the same wiki as the user last picked.

This doesn't seem functional yet. The user has to pick a wiki every time.

  • The category wiki input seems to be pre-populated with the first wiki in the event if there are multiple wikis - example. This should not happen - the input should be blank like in this case.

@MusikAnimal Thank you! This looks pretty good to me on the frontend. We should do some stress testing on this to see if we need any limits.

@Niharika - Did you want to assign someone to QA this?

@Niharika - Did you want to assign someone to QA this?

I would love to. Is there someone we can assign this to? I've been meaning to do it myself but I have other things on my plate.

I would love to. Is there someone we can assign this to? I've been meaning to do it myself but I have other things on my plate.

Unfortunately, all the QA Engineers are already stretched pretty thin and (as you know) CommTech doesn't have any dedicated QA resources. But I was thinking maybe you could get Sati to help QA (and/or another developer).

I would love to. Is there someone we can assign this to? I've been meaning to do it myself but I have other things on my plate.

Unfortunately, all the QA Engineers are already stretched pretty thin and (as you know) CommTech doesn't have any dedicated QA resources. But I was thinking maybe you could get Sati to help QA (and/or another developer).

Sati is on a sabbatical.

I'll give this a go later this week.

@MusikAnimal, please let me know when I can start testing this out (and get representative results as to speed, etc.) Thanks.

@MusikAnimal Found one bug so far:

image.png (377Γ—844 px, 56 KB)

It doesn't use the last wiki chosen but holds on to the first one picked by user.

@MusikAnimal Found one bug so far:

image.png (377Γ—844 px, 56 KB)

It doesn't use the last wiki chosen but holds on to the first one picked by user.

Fixed!

Thanks @MusikAnimal. This looks good to me. @jmatazzoni found that category redirects aren't observed in the tool. We should make a new ticket for that.
I think we're good to ship categories to production. There are some UI message improvements we can make. I'll leave that ball in Joe and Prateek's court. :)

I see a number of small UI issues. I can file tickets for them. But is there any plan to user test this?

I see a number of small UI issues. I can file tickets for them. But is there any plan to user test this?

Feel free to create tickets, or even comment here, if they're very small.

We're redoing the whole UI soon anyway, right? So maybe we don't need to bother with user testing and such?

I see a number of small UI issues. I can file tickets for them. But is there any plan to user test this?

Not really. We made the mistake of not recruiting a few test users when the project began. I don't think it's too late to rectify this with the ongoing Event Metrics consultation though so in future we can ask users to help test new features on staging.

MusikAnimal moved this task from QA to Q2 2018-19 on the Community-Tech-Sprint board.

I'm assuming this can be resolved since it's been deployed.