[Freeipa-devel] [PATCH] Big webUI patch.

[Adam Young]

On 09/16/2010 06:53 AM, Pavel Zuna wrote:
> On 09/15/2010 03:41 PM, Adam Young wrote:
>> On 09/15/2010 08:46 AM, Pavel Zůna wrote:
>>> Re-based version of the patch attached, that should apply on the
>>> current master. It doesn't have the Javascript library files (BBQ,
>>> jQ-UI). This makes the patch a lot smaller and easier to process.
>>>
>>> I'm going to post another patch that does nothing, but adds the
>>> library files.
>>>
>>> Pavel
>>
>>
>> Things to add to the to do list before this can go in:
>>
>> Netgroups is missing most of their associations
> Fixed.
>
> The menu leading to associations was only generated using the 
> 'memberof' attribute. Now it uses all attributes in 
> LDAPObject.attribute_members.


Ha:  You probably just made things both easier and harder in a single 
stroke.  Easier for implementing new associations, but harder in that 
many of the existing associations had strange rules.  We also have 
several associations that we hadn't implemented yes, like the 
association between users and roles, as we don't have roles.  We'll need 
a way to hide stuff until we work out the kinks.  Endi, take note.


>
>> netgroup_show.json has unmerged changes. We should revert to the version
>> in the top of tree
> Fixed.
>
> netgroup_show.json changes got in by mistake.
>
>> You have removed the author from several files and replaced it with only
>> your own.
>
> Only in two files were it made sense:
> 1) add.js, because I had to rewrite it from scratch completely.
> 2) details.js, because I started on a reverted version, that didn't 
> have your changes (and name) yet. The changes I'm talking about were 
> the addition of DetailsForm function, about 20 lines of code (out of 
> witch 10 were copy pasted from somewhere else).
>
> I added you as the author on search.js even though it's also pretty 
> much a complete rewrite, but I felt that it was based on your ideas. I 
> also didn't touch authorship info on associations even though I made 
> more or less
> significant changes to it.
>
> I just acted naturally without thinking about it when adding the file 
> headers. If you think it's unfair and I forgot to mention you or Endi 
> somewhere, I'm sorry and we can fix it no prob.

No problem.

You should feel comfortable adding your name as an author to 
associations.  You've made significant contributions there.

Details is pretty much your work, so I feel comfortable letting you take 
complete bla^H^H^Hownership.  I suspect that if we look really closely 
at add.js, we can incorperate it into details, as it really should be a 
minor variation in functionality.  Long term, it would be a logical 
pattern to have  add and edit for things in the same file.  We already 
do that with associations.

The ones that were a refactoring of our work should still contain us as 
authors.  I think that is goingto be a fairly common pattern moving 
forward:  as we get better and better understand of stuff, we should 
feel comfortable simplifying the implementations.  Most refactorings can 
be automated (at least in som languages) and don't constitute a major 
change in the conceptual underpinnings.  Just like when the the US 
version of Harry POtter comes out, and they CHange "Jumper" to "Sweater" 
and "Garden" to "Back-yard."   I don't want to make anyone wary of 
making necessary changes due to authorship issues.  Once someone's name 
goes on a file as author, we should leave it there unless there are 
legal reasons to remove it.


You have a good point that you make in the comments about the use of 
stuff from seearch.js inside of associations.   PLaces where we come up 
with our own UI patterns should get pulled out into JQuery plugins, 
especially in the case where they get reused.  The file search.js  is 
really the union of two distinct things:  a business object for querying 
the server for lists of things based on a filter, and a results table.  
Moving forward, we might want to split it along these fault lines, and 
then the logic for binding them together goes into the webui.js file.






>
>>
>> I posted the diff:
>>
>>
>> https://fedorahosted.org/freeipa/attachment/ticket/41/pzuna-freeipa-0022-3-BIG.patch 
>>

New Diff posted here 
https://fedorahosted.org/freeipa/attachment/ticket/41/pzuna-freeipa-0022-4-BIG.patch
>>
>
> Pavel

Going through the new patch now.  I found one more issue since this 
review:  You need to add new files to Makefile.am in install/static.  
Specifially, entity.js is not there.