-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add callouts on round pages to move up #993
Add callouts on round pages to move up #993
Conversation
|
||
// loop through the needs, highest to lowest, and point them to the | ||
// one where they can make the most impact with their current access | ||
foreach (array_keys($backlog_stats) as $round_target) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious about the impact of the db call. Is this a popular page with this new db call that may impact other scenarios performance? I assume not, just a thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair question and I've thought about it. This is the same query that is used to generate the round backlog graphs that are in news items on the Activity Hub and the P1 round page and so it seems to be pretty quick. I'm already eyeing generalizing the query_graph_cache()
and making use of it here and on the My Recommendations page, but I don't want to optimize too soon.
// prepare the common wordings between the callouts | ||
$please_move_up = sprintf( | ||
_("Please consider proofreading a project in $round_target. Having eligible users working in <a href='%s'>%s</a> helps significantly with our backlog."), | ||
"$round_url$round_target", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we html attribute encode here?
$please_move_up = sprintf( | ||
_("Please consider proofreading a project in $round_target. Having eligible users working in <a href='%s'>%s</a> helps significantly with our backlog."), | ||
"$round_url$round_target", | ||
$round_target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we control all the strings here, but may make sense to html encode in case any funcky localization issues pop upl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's hard because if we html_encode()
it, the links HTML will get escaped and not render as links.
if ($round->after_satisfying_minima == 'REQ-AUTO') { | ||
echo sprintf( | ||
_("Simply go to the <a href='%s'>%s</a> page to be automatically granted access, find a project that interests you, and start proofreading."), | ||
"$round_url$round_target", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above and throughout in html string generation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Quick first pass, not detailed testing (and I haven't read through the code in detail, which would answer the question, I'm sure) -- are the F1 and F2 callout boxes supposed to be recommending proofreading in P3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because it's always going to recommend people to the round with the highest backlog (that they can work in), which the sandbox thinks is P3 per the ["P3", "F2", "F1", "P1", "P2"]
hack mentioned in the description. That's also true for PROD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that applies to my next point of confusion (comment below) -- I may have "access" to F1 now, but P1 still has the largest backlog on TEST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code works as advertised, but some of the callout placements confuse me.
As I mentioned earlier, with my main username (which has access to all rounds), no matter which round page I looked on (except P3, which did not have a callout box at all), I was directed to P3 (even from F1 and F2).
I just took a brand new user (well, not in days since registration, but in that it hadn't done any pages yet), proofed 10 pages to unlock P2 and F1, did the formatting quiz pages, and went to the F1 page, only to be directed back to P1. Which also confused me.
Bear in mind that I haven't traced down all of the logic in the code.
|
||
// don't encourage people towards $ELR_round, as that isn't really | ||
// helpful for most sites | ||
if ($round->id == $ELR_round->id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srjfoo this line should have prevented the code from ever referring you to P1 (the entry level round) regardless if that's where the biggest backlog was or not. Which user were you testing with and I'll see if I can piggy-back on that account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing user crickets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's it.
It makes sense that both can't show up at the same time. Perhaps, in addition to doctoring $page_tally_threshold
in the sandbox, what defines a new user should also be tweaked to align the two so that the testing results will align better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have tons of code in activity_hub.php
, gradual.inc
, and round.php
that uses the number of pages in P1 (and only P1) to show or reveal content and this falls under that category. Somewhat separately, the "proof in higher round" callout in this PR and that code should not be shown together, but they operate on different sources.
I will noodle over how to make this less fragile and hopefully more preditable.
Scope creep ... Would it be possible to consider also genre? or possibly the longest time spent in the round? From my very small experience PMing and looking at the "neglected projects" page, projects of a technical or military nature get through P3 in a reasonable time but sometimes spend several years in F2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking for a moment about what we want, not about what the code can/can't easily do...
- We want the recommendations to guide people to where they are most needed
- We want to make it as likely as possible that people will follow the recommendations.
- We don't want the recommendations (if acted upon by users) to cause pressure on people's resources.
I feel this change addresses point 1 more effectively than point 2. In order to address point 2, I think it might be better to acknowledge the difference between F rounds and P rounds and that there is a "progression" of difficulty/experience within each of those groups of rounds.
Maybe it would be helpful to consider specific user qualifications and think what we would want to recommend to them when they attempt to work in a specific round. By "better" in the text below, I mean not only "is there a greater need there" (point 1 above) but also "is the user likely to take note of the recommendation" (point 2 above)
For example, suppose a user can work in any round, and they go to do some F1 work. Would it be "better" to recommend them to work in P3 (most need) or F2 (most need in the F rounds)? If they prefer formatting to proofing or just fancied some formatting today, they may ignore any recommendation to work in P3, but may be prompted to move to F2.
If I understand correctly, point 3 above is addressed by not recommending P1 to anybody, which could cause pressure on the CP/PM.
Genre and time-in-round are important when selecting a project within a round, but aren't so useful in pointing them towards a round. Genre, time-in-round, and overall age are used in the new My Recommendations page which is able to incorporate more information about the user's accessible rounds too.
I'd actually start with a number 0:
Recall that if you're a new user and proofreading away you might not readily know there is a P2 and if and how you qualify for it. And then the same about P2 -> F1. Until you have access to a round they don't even show up as links in the navbar. I agree with your points, people will self-select based on the type of work they want to do (P vs F) and if the "do work in " message is always shown, that isn't helpful and will eventually just train people to ignore callouts. Strictly using the type-of-work isn't going to help our number 0 goal though, because if we really want people to work in P3 we need them to work in F1 at some point as that's a prerequisite. And the need for work P3 is very real -- ~50% of all incomplete pages in all rounds at pgdp.net are in P3. So we want to encourage people to higher rounds and to those qualified where the most work is but not at the expense of telling them something they don't actually care about and won't do. |
What if for rounds you were qualified for, the dialog prompted both the round with the biggest backlog (what it does now) plus the biggest backlog round in the type-of-work of the page you're on. So if you're on the F1 page it would prompt you to P3 and F2. And if you're on the P2 page it would prompt you to P3 -- and assuming the biggest backlog has moved to F2 -- to F2? |
I've been thinking about this, and, going back through the comments others have made, and copying/quoting comments in bits and pieces ...
Some P2s are not ever going to qualify for P3, no matter how much they want to, and some F1s are not ever going to qualify for P3 or F2, no matter how many times they apply, so if they've applied and been turned down 3 or 4 or 5 times, the callout boxes urging them towards higher rounds are just going to be either annoying, depressing, or offensive. So far, in 2023, 62 apps for P3; 16 approved. Instead of showing it for everyone who is close to qualifying, would it be possible to check the access_log and not show the message if they've applied (and been denied) within the past year or two, or even 3? Some of those who reapplied for P3 this past year were well aware, and didn't need any prompting at all. In addition, there may be those who are just not interested. Leaving the callout boxes in place for those users just encourages them to skip down the page without reading. How difficult would giving users the ability to hide the callout box be to implement?
Yes, it is, but by the time users get to P2, they're pretty aware that P3 exists.
As I understand it, P1 is not recommended for anyone except those who have proofed fewer than 300 pages AND have been registered for less than 21 days? So, if on PROD they land on a round page they're not qualified for, they probably do need to be redirected back to P1 -- or to SR.
Dialog or callout? (I'm assuming callout, but this was slightly confusing) Apologies for the disjoint nature of this response. |
Very probably. That table is indexed by user so it should be quick to query. I'd probably do this in This also caused me to wonder if we should be drastically increasing the number of P2 pages required for P3 and the time on site. In the past decade has any person actually qualified for P3 after doing only 150 pages in P2? And in 42 days of joining the site? That seems really low compared to the 300 pages to move from P1 -> P2.
Not all that difficult. We could store it as a UserSetting and even have it time out after a while (90 days? a year?). I'm thinking we do it per round encouragement, so that if the message was "try proofreading in F1" and they dismissed it, the encouragement to F1 would not show up for $some_time but if it would otherwise encourage them to a different round -- say they proofed in F1 and now are eligible for F2 -- those would still show unless they too are dismissed. If we think the ability to dismiss for $some_time would mitigate how annoying the dialog would be I'm happy to implement that, we just need to figure out how long the dismiss would stick around for.
Sorry, dialog == callout in that sentence. |
👍
That sounds like a good idea, too, but I think that it would be a good idea to consult with the P3 and F2 evaluators to see what their opinion is on number of pages, and time on site before first application.
I'd probably opt for 6 months, but would be okay with 90 days. I do think that would mitigate the annoyance. And yes, I think that dismissing would be best applied to separate messages individually, if messages are stacked. |
Based on the feedback I'd like to:
Future work that would not be gated on the code in this PR getting pushed to production:
I'll get to work on the first bullet for this PR here soon. |
Add callouts to the round pages to encourage users to work in rounds with the largest backlog.
da89a61
to
52e6813
Compare
Rebased locally to resolve conflict and force-pushed the new branch up. Will merge in after the CI/CD passes. |
Add callouts to the round pages to encourage users to work in rounds with the largest backlog. This is a generalized, and improved, version of the 2b891bd commit in the
pgdp-production
branch which only applies to P1. This version will encourage users towards the round with the biggest backlog that they have access to or are eligible for except for quizzes.We may want to consider updating the version of this that goes into
pgdp-production
to exclude the bits about applying for P3/F2 if they are otherwise eligible.This is testable in https://www.pgdp.org/~cpeel/c.branch/encourage-higher-rounds/
The above sandbox has a hack applied to simulate the backlog order on PROD: P3, F2, F1, P1, P2 rather than what's on TEST which has the biggest backlog in P1: