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

[Christian W. <cw...@cw...>]

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  :)

> 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"]'

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'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 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 :)


Btw, how did you fix the time-problematic unit tests now? Do they run
for you now?

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


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.

-- 
Regards/Mit freundlichen Grüßen
Christian Weiske

-=≡ Geeking around in the name of science since 1982 ≡=-