[cle-release-team] Review for SAK-23143

[Matthew Jones]

Yeah, I considered it, but there was no NPE checks in the original code,
which was essentially doing all the same stuff. I'll check these out it
looks like in every case for Sakai, it follows the best practice of
returning an empty new empty collection rather than a null, so any
additional is just way over checking. And if the value was null from an
object in the session, I don't know of what the "else" case to fix that
would be. Ideally java would finally have an Elvis operator, unfortunate
that it isn't included included in the language. I think it probably would
have saved millions of hours of code problems though across all developers.
;)




On Mon, Mar 10, 2014 at 1:39 PM, Bryan Holladay <holladay at longsight.com>wrote:

> Looks good... straight forward.  I added tons of NPE spam to that crucible.
>
>
> On Fri, Mar 7, 2014 at 4:07 PM, Matthew Jones <matthew at longsight.com>wrote:
>
>> I put in a fix for this blocker for sites being accidentally deleted and
>> would appreciate a review/testing if anyone has any time. I'd hate it to
>> accidentally delete worse than it is right now. ;)
>>
>> https://crucible.sakaiproject.org/cru/SAKTRUNK-43
>>
>> This is looking pretty good in my testing in both the debugger and
>> otherwise.
>>
>> Basically it does the suggestion of rather than storing the new property
>> in the session, it just adds it as a property, adds a site property of
>> "new" = "true"  to the site. This is removed when the site is saved. The
>> side advantage of this is that if someone closes the browser completely,
>> these orphaned pages would be easier to find.
>>
>> There were are also 2 state attribute to keep track of both new page and
>> tool creation. The complication slight of these is that they can be cleaned
>> up pretty easily when you hit done, but if you click save instead then the
>> code has to go through all the pages and all the tools of the site cleaning
>> up this property. I added that code and it looks to be working efficiently
>> (the pages/tools don't appear to be written until the final save anyway,
>> unlike site) but just wonder about it.
>>
>> Would someone be likely to accidentally remove a page from a site using
>> multi tabs? Probably less likely. And there doesn't seem to be a cancel
>> button for a page/tool anyway, it's either done, save or cancel everything.
>> So you probably can't mess that up, so maybe not even a need to track
>> page/tool properties right now. (Unless I'm missing the cancel button)
>>
>> When testing, follow the description. Also go back into the new site
>> after saving and verify the property "new" is removed. I'd go into the
>> pages and make sure there are no "new" properties there either.
>>
>> _______________________________________________
>> cle-release-team mailing list
>> cle-release-team at collab.sakaiproject.org
>> http://collab.sakaiproject.org/mailman/listinfo/cle-release-team
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://collab.sakaiproject.org/pipermail/cle-release-team/attachments/20140310/6bb6663a/attachment-0001.html