[Building Sakai] SiteService.getSiteSkin and neoportal

[Aaron Zeckoski]

I disagree. The javadocs in much of the kernel are inaccurate and if
we changed all the APIs to match the javadocs we would make life hell
for most of the developers in this project. The current behavior is
the API and the javadocs should be switched instead of doing it the
other way around (at least, this is what we have done in the past when
this has come up).

Either way, like I said, create a JIRA and let's get it on the topics
for the CLE team meeting this week.

-AZ


On Tue, Sep 4, 2012 at 8:29 PM, Steve Swinsburg
<steve.swinsburg at gmail.com> wrote:
> If those are the javadocs then that is what it should do, so it should
> return the default skin specified via skin.default
>
> Query re the patch. Should the neo- be specifically prepended? Can't we just
> have this:
>
> skin.default=neo-default
>
> cheers,
> Steve
>
>
>
> On 05/09/2012, at 1:46 AM, Adrian Fish <adrian.r.fish at gmail.com> wrote:
>
>
>
>>
>> 1) We generally do not change API behavior unless there is a really
>> good reason. In this case the change results in a loss of information
>> (i.e. there would no longer be a way to determine if a site has no
>> skin set and therefore is using the default skin, in other words,
>> assigning a site the same skin as the current default one and having
>> no skin assigned will appear to be the same).
>
>
> Is anybody actually using it in that way? The docs for getSiteSkin state
> this:
>
> 'Compute the skin to use for the (optional) site specified in the id
> parameter. If no site specified, or if the site has no skin defined, use the
> configured default skin.'
>
> This implies it should always return something whereas under neoportal it
> returns null.
>
> Cheers,
> Adrian.
>
>>
>>
>>
>> On Tue, Sep 4, 2012 at 11:05 AM, Adrian Fish <adrian.r.fish at gmail.com>
>> wrote:
>> > Hi Aaron,
>> >
>> > I don't know if there is a JIRA.
>> >
>> > This isn't changed behaviour; I think it's always behaved like that.
>> > I've
>> > basically migrated a 2.8 db to 2.9.x and set all of our explicitly
>> > skinned
>> > sites to blank skins. In other words they should get the default now and
>> > in
>> > most cases they do perfectly well. The problem lies with tool logic that
>> > calls getSiteSkin and expects to get something other than null, which,
>> > in a
>> > system where a default skin always applies, seems reasonable. I've only
>> > spotted this since removing the explicit skins from sites.
>> >
>> > Obviously, there would be ramifications for tools that handle the
>> > possibility of null being returned if getSiteSkin respected the default
>> > skin
>> > and returned it.
>> >
>> > Cheers,
>> > Adrian.
>> >
>> >
>> > On 4 September 2012 15:44, Aaron Zeckoski <azeckoski at unicon.net> wrote:
>> >>
>> >> Adrian,
>> >> Is there a JIRA related to this?
>> >>
>> >> Also, this doesn't sound like changed behavior but I could be mistaken
>> >> there. Are you saying this used to be the case but that it changed and
>> >> if so, when do you think it changed?
>> >> -AZ
>> >>
>> >>
>> >> On Tue, Sep 4, 2012 at 10:16 AM, Adrian Fish <adrian.r.fish at gmail.com>
>> >> wrote:
>> >> > When setSiteSkin is called it returns null if a skin has not been
>> >> > explicitly
>> >> > set on a site. Is this the expected behaviour? Shouldn't it return
>> >> > the
>> >> > name
>> >> > of the default skin as set by the skin.default property? Shouldn't it
>> >> > also
>> >> > prefix the skin with 'neo-' if 'portal.templates=neoskin' is set?
>> >> >
>> >> > The reason I'm asking is that I've always depended on getSiteSkin
>> >> > returning
>> >> > the actual skin name that is in use; now I've stopped explicitly
>> >> > specifying
>> >> > skins on sites, tools like YAFT, CLOG and ROSTER2 don't get styled.
>> >> > It
>> >> > seems
>> >> > odd having to implement logic in a tool to massage the skin
>> >> > appropriately
>> >> > when it could happen in getSiteSkin. SiteStats also implements some
>> >> > logic to
>> >> > sort the skin name out.
>> >> >
>> >> > I've attached a patch which mods BaseSiteService in the way I
>> >> > describe.
>> >> >
>> >> > Cheers,
>> >> > Adrian.
>> >> >
>> >> > _______________________________________________
>> >> > sakai-dev mailing list
>> >> > sakai-dev at collab.sakaiproject.org
>> >> > http://collab.sakaiproject.org/mailman/listinfo/sakai-dev
>> >> >
>> >> > TO UNSUBSCRIBE: send email to
>> >> > sakai-dev-unsubscribe at collab.sakaiproject.org
>> >> > with a subject of "unsubscribe"
>> >>
>> >>
>> >>
>> >> --
>> >> Aaron Zeckoski - Software Architect - http://tinyurl.com/azprofile
>> >
>> >
>>
>>
>>
>> --
>> Aaron Zeckoski - Software Architect - http://tinyurl.com/azprofile
>
>
> _______________________________________________
> sakai-dev mailing list
> sakai-dev at collab.sakaiproject.org
> http://collab.sakaiproject.org/mailman/listinfo/sakai-dev
>
> TO UNSUBSCRIBE: send email to sakai-dev-unsubscribe at collab.sakaiproject.org
> with a subject of "unsubscribe"
>
>



-- 
Aaron Zeckoski - Software Architect - http://tinyurl.com/azprofile