Re: [Semanticscuttle-devel] getBookmarks() tag problems / user session problems

[Mark P. <mpe...@gm...>]

Hey Christian,

On Tue, Jun 7, 2011 at 3:08 PM, Christian Weiske <cw...@cw...> wrote:
> Hello Mark,
>
>
>> Didn't get a reply from my previous e-mail, so I went ahead and made
>> the assumption to #6 and everything else should be ok.  Forgive me on
>> my poor response, just been crazy after vacation.
> I didn't do anything on SemanticScuttle the last week, but didn't
> forget you. I began reading Winnetou I and am at the end of Winnetou
> II  :)
Interesting.  I've never heard of the book, but it's obviously a very
popular series (selling over 200 million copies).  I may head down to
our local used book store to see if they have it.  And not that it
means much, but I'm like a fraction of Cherokee Indian.  For that
matter, I also have some German heritage too... :)

>
>> Also, historically
>> I've not done much unit testing, but as I'm learning through this the
>> unit testing can prove quite valuable.
> One test fails for me; it's
>
> 1) www_bookmarksTest::testVerifyPrivateRSSLinkExists
> Number of Links in Head not correct
> Failed asserting that <integer:4> matches expected <integer:5>.
> /home/cweiske/Dev/semanticscuttle/cwdev/tests/www/bookmarksTest.php:102
>
> Btw, just checking for a number of links in the head isn't the best
> option. Add some xpath to count the rss links only.
> Instead of
>> '//ns:link'
> use something like
>> '//ns:link[@rel="alternate" and @type="application/rss+xml"]'
I've updated the xpath and did some additional checking.  Forgive my
inexperience with xpath.  Still on a learning curve with it.  I'm
loving it though!  I've also added a file for index.php.

>
> Did you also add negative tests? i.e. no private feed links when the
> user doesn't have a private key or he isn't logged in? (didn't look yet)
I didn't, but I have now.

>
> I'm also not fully happy with the duplicated code in rss.php (the
> privatekey login and check if that didn't work), but could live with it
> for now.
I've looked this code over quite a bit and I could pull a couple lines
of code out to remove some of the duplication, but I think that whole
if/else section may need to be completely reworked and I don't feel
comfortable (yet) to do that.  Let's get this revision out and then we
can rework it.

>
>
>> I think I've added what you
>> requested, so let me know if you see anything else.  I'm a bit of a
>> novice on xpath but was able put it together.
>>
>> I've also merged with master.  :)
> Seems the merge didn't fully work; the rss icon on the pager doesn't
> work anymore since it tries to use the old images/rss.gif location.
> Don't worry, that happens to the best :)
Fixed!  Duh!  :)

>
>
> Btw, how did you fix the time-problematic unit tests now? Do they run
> for you now?
After reverting back to gmdate(), I think there are only two changes
that was necessary.  And yes, they run perfectly for me.  Please
verify that I've done the right thing.
1) In tests/Bookmark2TagTest.php, in testGetPopularTagsDays(), the use
of "today" doesn't return time (defaults to 00:00:00 AM) so I changed
it to now which does.
2) In Service/Bookmark2Tag.php in getPopularTags(), there was a date()
call that was never migrated over to use gmdate().

>
> Oh, and I'd like to see smaller commits. For example, the reversion of
> the gmdate changes should be a single commit.
Understood.  I completely understand the usability of smaller commits.

>
>
> In www/index.php
>> filter($sitename . sprintf(T_(': Recent bookmarks (private)')) .
>> $currentUsername),
> make the full string translatable, including the user name:
>> T_(': Recent bookmarks (private) %s')
> (there was a space missing after the ) btw)
> and give the user name to sprintf as parameter - the sprintf was
> totally useless since there were no placeholders in the string.
Updated.  Please review to make sure it is ok.

Cheers,
Mark

>
> --
> Regards/Mit freundlichen Grüßen
> Christian Weiske
>
> -=≡ Geeking around in the name of science since 1982 ≡=-
>
> ------------------------------------------------------------------------------
> EditLive Enterprise is the world's most technically advanced content
> authoring tool. Experience the power of Track Changes, Inline Image
> Editing and ensure content is compliant with Accessibility Checking.
> http://p.sf.net/sfu/ephox-dev2dev
> _______________________________________________
> Semanticscuttle-devel mailing list
> Sem...@li...
> https://lists.sourceforge.net/lists/listinfo/semanticscuttle-devel
>
>