denise: Image: Me, facing away from camera, on top of the Castel Sant'Angelo in Rome (Default)
Denise ([staff profile] denise) wrote in [site community profile] dw_dev_training2009-07-29 10:27 pm

Bug Walkthrough: Bug 1512

I decided to take on Bug 1512 as a quick hit, and thought I'd do a walkthrough so people could see my process!

(I like having walkthroughs. I think we should do them more often!)



1. I looked at the bug description, which came from [site community profile] dw_suggestions. It seems reasonably clear, and easy enough to implement, so it seemed a good candidate for my "quick fix". I set the status to Assigned, and put myself in the assignee field.

2. First, I look at the page in question ( http://www.dreamwidth.org/manage/circle/invite.bml ). To find the code I'm going to be looking at, I could read through the entire code to find the one section I need to change -- the text of the header of the page -- but why bother? There's an easier way to find where in the code you need to look, and that's by using ?uselang=debug. This is a good technique if you're looking at a page and need to find what section of the code generates it, like so:

http://www.dreamwidth.org/manage/circle/invite.bml?uselang=debug

Viewing that, it tells me that the text string I need to be working with is .intro.code.

3. I go over to my 'hack and open the files ~/dw/htdocs/manage/circle/invite.bml and ~/dw/htdocs/manage/circle/invite.bml, since those are the files I'm going to be working with. (My development process involves using BBEdit's "open file from ftp server" function as an editor, so I'm working on my local machine and can have both files open side-by-side.)

4. Using my editor's Find function, I find this section of code (line numbers added for ease of reference):

   188    my $findfriends_intro = LJ::run_hook("findfriends_invite_intro");
   189    if ($findfriends_intro) {
   190        $body .= $findfriends_intro;
   191    } elsif ( $LJ::USE_ACCT_CODES ) {
   192        $body .= "<?p " . BML::ml('.intro.code', { aopts => "href='$LJ::SITEROOT/manage/invitecodes.bml'" } ) . " p?>";
   193    } else {
   194        $body .= "<?p " . BML::ml('.intro', { aopts => "href='$create_link'", createlink => $create_link} ) . " p?>";
   195    }


Line 192 looks promising, since that has the text string I want to change.

5. I know that, since I'm going to be changing the text string, the best way to handle that is to create a new text string with a number on the end (.intro.code2), changing the reference in invite.bml to the new string name and adding the new string in the .bml.text file. This is because there's no way to tell the translation system (*ritual gestures of warding*) that the string has been updated, because otherwise it might overwrite work that the site copy team has done.

So, the first thing I do is change
.intro.code
to
intro.code2
in both invite.bml and invite.bml.text. I always do that first, because if I don't, I know there's a pretty good chance that I'm going to forget to. (Why, no, this hasn't bitten me in the past before.)

6. I'm going to start by changing the text string, so I know what I want it to say before I figure out the code bits to support it. So, in my editor, I change:

.intro.code=Use this form as often as you like, or grab a set of links at once from <a [[aopts]]>your complete list of invite codes</a>. Note that invite codes which have been sent but not yet redeemed are still counted as unused.

To the tentative new version:

.intro.code2<<
You can use this form as often as you'd like. Invite codes that you've sent, but haven't been used to create accounts yet, are counted as unused.

You currently have [[num]] unused invite codes available. The first five are displayed here. For the full list, or to get multiple codes at once, see <a [[aopts]]>your complete list of invite codes</a>.
.

(The pedantic among us will note the grammar tweaks. *g*)

The reason I change it to use the << multi-line version of the translation string is so that I can paragraph-break it without having to make two new text strings, which would be annoying.

7. So, now I know I need to figure out what the number of unused invite codes are, so I can pass that value to [[num]] in the string. (For a quick introduction to the joys of the translation system, check out the Wiki page on English-stripping.)

This seems like such a useful function that I just know it has to exist somewhere. So I flip over to a new browser window and open up the web Mercurial interface for the code (I like doing it that way because it doesn't require me to have eighty editor windows open to trace some code back to its source; you might prefer to look at code through your 'hack.) I know that invite codes are a DW-specific thing, not something we inherited from LJ, so it's a pretty good bet the invite code-related functions are going to live in cgi-bin/DW. Sure enough, in cgi-bin/DW/InviteCodes.pm, there's a promising-looking function:

   337=head2 C<< $class->unused_count( user => $userid ) >>
   338
   339 Returns a count of unused invite codes owned by $userid.
   340
   341 =cut
   342
   343 sub unused_count {
   344    my ($class, %opts) = @_;
   345    my $userid = $opts{userid};
   346
   347    my $dbr = LJ::get_db_reader();
   348    my $count = $dbr->selectrow_array( "SELECT COUNT(*) FROM acctcode WHERE userid = ? AND 
                  rcptid = 0", undef, $userid );
   349
   350    return $count;
   351 }


(The odd-looking syntax at the top of that means that it's in perldoc format, which means that the code and its documentation are in the same file.)

That looks like exactly what I want, so I change line 192 to read:

$body .= "<?p " . BML::ml('.intro.code2', { aopts => "href='$LJ::SITEROOT/manage/invitecodes.bml'" , num => DW::InviteCodes->unused_count( $u )} ) . " p?>";


Save and hit up the page I just changed on my hack.

8. Well, that almost worked. The page is now showing me my new changed text, and there's a number on the page, but the number is 0, and the account I'm logged on as has four unused invites. That means that the wrong value is being passed to '[[num]]'. The first thing I think is that maybe BML::ml doesn't like having a separate function called as part of the variable assignment, so I separate it out:

my $unusedinvites = DW::InviteCodes->unused_count( $u );
$body .= "<?p " . BML::ml('.intro.code2', { aopts => "href='$LJ::SITEROOT/manage/invitecodes.bml'" , num => $unusedinvites } ) . " p?>";


That doesn't work either, though; it's still showing 0.

9. I look back at cgi-bin/DW/InviteCodes.pm. Aha! I say. The documentation says it should be called as $class->unused_count( user => $userid ). So I change it to match in my file:

my $unusedinvites = DW::InviteCodes->unused_count( user=>$u );
$body .= "<?p " . BML::ml('.intro.code2', { aopts => "href='$LJ::SITEROOT/manage/invitecodes.bml'" , num => $unusedinvites } ) . " p?>";


Still no dice, though. Hrm.

10. At this point, I'm stumped at what I'm doing wrong. I know that the problem is with how I'm calling unused_count, and I'm guessing that the problem has something to do with me passing the wrong parameters to the function.

When I get stuck like that, the first thing I do is to look for how it's done in other places, to see if I can figure it out from context. So I go and check with rgrep for other instances of that function being called:

dh-rahaeli@hack:~/dw/cvs/dw-free$ rgrep -i unused_count * | more

cgi-bin/DW/Hooks/Display.pm: my $numinvites = DW::InviteCodes->unused_count( userid => $u->id );
cgi-bin/DW/InviteCodes.pm: my $count = DW::InviteCodes->unused_count( userid => $userid );
cgi-bin/DW/InviteCodes.pm:=head2 C<< $class->unused_count( user => $userid ) >>
cgi-bin/DW/InviteCodes.pm:sub unused_count {
cgi-bin/DW/BusinessRules/InviteCodeRequests.pm: my $unused_count = DW::InviteCodes->unused_count( userid => $userid );
cgi-bin/DW/BusinessRules/InviteCodeRequests.pm: return 0 if $unused_count;
htdocs/admin/invites/requests.bml: $cts{$u->id} = DW::InviteCodes->unused_count( userid => $u->id );
htdocs/admin/invites/review.bml: my $unused_count = DW::InviteCodes->unused_count( userid => $u->id );
htdocs/admin/invites/review.bml: $ret .= "p Number of unused invites: $unused_count p?";


That actually tells me a lot: it tells me that I don't need to pass the $u object to the function, I need to pass the userid of the $u object. So I look at those pieces and see which one most closely matches what I'm trying to do here, and try that instead:

my $unusedinvites = DW::InviteCodes->unused_count( userid => $u->id );
$body .= "<?p " . BML::ml('.intro.code2', { aopts => "href='$LJ::SITEROOT/manage/invitecodes.bml'" , num => $unusedinvites } ) . " p?>";


I reload, and ... Success!

11. Except the multi-line version of the text string is being all run together on the page I'm looking at, and I realize that just because there's multiple lines in the text file, it doesn't mean that they'll translate to the page itself. So I tweak the text string to include some linebreaks. While I'm at it, the number of unused invite codes doesn't quite pop, visually, so I add some formatting there, too:

.intro.code2<<
You can use this form as often as you'd like. Invite codes that you've sent, but haven't been used to create accounts yet, are counted as unused.<br /><br />

You currently have <strong>[[num]]</strong> unused invite codes available. The first five are displayed here. For the full list, or to get multiple codes at once, see <a [[aopts]]>your complete list of invite codes</a>.
.


Reload, and it's perfect -- to me, at least! So, time to generate the patch and upload it.
yvi: Dreamsheep Teal'c (Dreamsheep - Sheepl'c)

[personal profile] yvi 2009-07-30 07:59 am (UTC)(link)
I go over to my 'hack and open the files ~/dw/htdocs/manage/circle/invite.bml and ~/dw/htdocs/manage/circle/invite.bml

You mean ~/dw/htdocs/manage/circle/invite.bml.text , right :P

5. I know that, since I'm going to be changing the text string, the best way to handle that is to create a new text string with a number on the end (.intro.code2), changing the reference in invite.bml to the new string name and adding the new string in the .bml.text file. This is because there's no way to tell the translation system (*ritual gestures of warding*) that the string has been updated, because otherwise it might overwrite work that the site copy team has done.

Oh, I didn't know that!

Still no dice, though. Hrm.

Gosh, how I know the feeling :)

Nice walk-through, I always love them. I have one in the works for http://bugs.dwscoalition.org/show_bug.cgi?id=1454 .
afuna: Cat under a blanket. Text: "Cats are just little people with Fur and Fangs" (Default)

[personal profile] afuna 2009-07-30 08:21 am (UTC)(link)
(this was going as a reply to the entry, but...)

+1 more walkthroughs!
yvi: Kaylee half-smiling, looking very pretty (Default)

[personal profile] yvi 2009-07-30 08:24 am (UTC)(link)
Mine is staggering a bit because I had this perfectly logical way to fix the bug, with lots of explanations and everything... and then it was pointed out that it could be fixed so much easier :) So now I have two sets of perfectly logical explanations for how to fix it and a big "think easier!" ;)
afuna: Cat under a blanket. Text: "Cats are just little people with Fur and Fangs" (Default)

[personal profile] afuna 2009-07-30 08:25 am (UTC)(link)
I love it when that happens *G* (Have had that happen to me as well!)
afuna: Cat under a blanket. Text: "Cats are just little people with Fur and Fangs" (Default)

[personal profile] afuna 2009-07-30 08:26 am (UTC)(link)
\o/

(Oh, and from the glance through of this walkthrough, what you have looks good. I should go review/commit it, if it isn't done by the time I get home later. Hmm)
baggyeyes: Mac-Keyboard (Keyboard)

[personal profile] baggyeyes 2009-07-30 12:41 pm (UTC)(link)
Thank you thank you thank you. I remember assigning something to myself, and going - now what? This walk through is immensely helpful.

I made a pdf of it so I can refer to it in teh thick of things. And memmed it. And bookmarked it.
sophie: A cartoon-like representation of a girl standing on a hill, with brown hair, blue eyes, a flowery top, and blue skirt. ☀ (Default)

[personal profile] sophie 2009-08-01 01:14 am (UTC)(link)
If it helps, there are a few other bug walkthroughs too! See http://dw-dev-training.dreamwidth.org/tag/bug+walkthrough for the full list.
aphenine: Teresa and Claire (Default)

[personal profile] aphenine 2009-07-30 09:17 pm (UTC)(link)
Thank you for the walkthrough. It was most helpful!