Hi Matt, 1/ I think this behavior cannot break things as the previous scenario was to replace the existing page and would have rather been considered as a bug (a navigation is replaced by another navigation). So I think it is quite good, but I may be wrong and I will discuss tomorrow with Trong to have his opinion. 2/ I think we should provide a way to control how merge appends. Here I am thinking about leveraging the priority value and using it to compare it to the loaded navigation that will be merged to and according to the result do the following: - extension priority < existing priority : merge before - same priority : replace (which means that this is the current behavior) - extension priority > existing priority : merge after 3/ your patch does not have any unit test and that precludes the addition to the codebase. On Oct 27, 2010, at 6:30 PM, Matt Wringe wrote: > Currently when we are using the extension mechanism, we do not properly > merge the child PageNodes. What happens if that if the child PageNode > already exists it will overwrite the old PageNode with the new one. > > Since PageNodes also has children PageNodes we are stuck in a situation > where we can only handle the top level of PageNodes tree. We cannot > currently use the extension mechanism to add a new page node anywhere in > the page node tree. > > Jira with patch which merges the child nodes together: > https://jira.jboss.org/browse/GTNPORTAL-1605 > > I would have committed this, but it does change the behaviour of how > this works. Ie if someone was using the extension mechanism to overwrite > an existing page node. > Is anyone using it in this manner? Do we need to add an extra option to > pageNavigation which will tell it overwrite or not overwrite when > merging? > > _______________________________________________ > gatein-dev mailing list > gatein-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/gatein-dev

what I mean is that we discussed today with Trong about it and both feature seems related and it would make sense to unify them to define the behavior when exectuing component plugin that would be:

1/ ignore 2/ merge 3/ replace

On Oct 28, 2010, at 10:22 AM, Julien Viet wrote: > further more I see you also created : > > https://jira.jboss.org/browse/GTNPORTAL-1376 / "NewPortalConfigListener should allow for overwriting of default values" > > is it related to this, if yes, how ? > > On Oct 27, 2010, at 6:55 PM, Julien Viet wrote: > >> Hi Matt, >> >> 1/ I think this behavior cannot break things as the previous scenario was to replace the existing page and would have rather been considered as a bug (a navigation is replaced by another navigation). So I think it is quite good, but I may be wrong and I will discuss tomorrow with Trong to have his opinion. >> >> 2/ I think we should provide a way to control how merge appends. Here I am thinking about leveraging the priority value and using it to compare it to the loaded navigation that will be merged to and according to the result do the following: >> >> - extension priority < existing priority : merge before >> - same priority : replace (which means that this is the current behavior) >> - extension priority > existing priority : merge after >> >> 3/ your patch does not have any unit test and that precludes the addition to the codebase. >> >> On Oct 27, 2010, at 6:30 PM, Matt Wringe wrote: >> >>> Currently when we are using the extension mechanism, we do not properly >>> merge the child PageNodes. What happens if that if the child PageNode >>> already exists it will overwrite the old PageNode with the new one. >>> >>> Since PageNodes also has children PageNodes we are stuck in a situation >>> where we can only handle the top level of PageNodes tree. We cannot >>> currently use the extension mechanism to add a new page node anywhere in >>> the page node tree. >>> >>> Jira with patch which merges the child nodes together: >>> https://jira.jboss.org/browse/GTNPORTAL-1605 >>> >>> I would have committed this, but it does change the behaviour of how >>> this works. Ie if someone was using the extension mechanism to overwrite >>> an existing page node. >>> Is anyone using it in this manner? Do we need to add an extra option to >>> pageNavigation which will tell it overwrite or not overwrite when >>> merging? >>> >>> _______________________________________________ >>> gatein-dev mailing list >>> gatein-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/gatein-dev >> > _______________________________________________ gatein-dev mailing list gatein-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/gatein-dev

can we clarify quickly that in a specification and then you implement it ? On Oct 28, 2010, at 3:51 PM, Matt Wringe wrote: > On Thu, 2010-10-28 at 10:31 +0200, Julien Viet wrote: > >> what I mean is that we discussed today with Trong about it and both feature seems related and it would make sense to unify them to define the behavior when exectuing component plugin that would be: >> > yes, they are both related, we need options to define how we want things > merged. > > >> 1/ ignore >> 2/ merge >> 3/ replace >> > 4/ remove > we may have the situation where an extension/new portal may not want > something set which is already set in the original. This would be > equivalent to replace with null. > > 5/ priority > we may want to insert something before or after what is already set (ie > page nodes is a good example of this). Although I don't know how you > would want to handle something like insert between these nodes, or > insert at position 4. > > > >> >> On Oct 28, 2010, at 10:22 AM, Julien Viet wrote: >> >> >>> further more I see you also created : >>> >>> https://jira.jboss.org/browse/GTNPORTAL-1376 / "NewPortalConfigListener should allow for overwriting of default values" >>> >>> is it related to this, if yes, how ? >>> >>> On Oct 27, 2010, at 6:55 PM, Julien Viet wrote: >>> >>> >>>> Hi Matt, >>>> >>>> 1/ I think this behavior cannot break things as the previous scenario was to replace the existing page and would have rather been considered as a bug (a navigation is replaced by another navigation). So I think it is quite good, but I may be wrong and I will discuss tomorrow with Trong to have his opinion. >>>> >>>> 2/ I think we should provide a way to control how merge appends. Here I am thinking about leveraging the priority value and using it to compare it to the loaded navigation that will be merged to and according to the result do the following: >>>> >>>> - extension priority< existing priority : merge before >>>> - same priority : replace (which means that this is the current behavior) >>>> - extension priority> existing priority : merge after >>>> >>>> 3/ your patch does not have any unit test and that precludes the addition to the codebase. >>>> >>>> On Oct 27, 2010, at 6:30 PM, Matt Wringe wrote: >>>> >>>> >>>>> Currently when we are using the extension mechanism, we do not properly >>>>> merge the child PageNodes. What happens if that if the child PageNode >>>>> already exists it will overwrite the old PageNode with the new one. >>>>> >>>>> Since PageNodes also has children PageNodes we are stuck in a situation >>>>> where we can only handle the top level of PageNodes tree. We cannot >>>>> currently use the extension mechanism to add a new page node anywhere in >>>>> the page node tree. >>>>> >>>>> Jira with patch which merges the child nodes together: >>>>> https://jira.jboss.org/browse/GTNPORTAL-1605 >>>>> >>>>> I would have committed this, but it does change the behaviour of how >>>>> this works. Ie if someone was using the extension mechanism to overwrite >>>>> an existing page node. >>>>> Is anyone using it in this manner? Do we need to add an extra option to >>>>> pageNavigation which will tell it overwrite or not overwrite when >>>>> merging? >>>>> >>>>> _______________________________________________ >>>>> gatein-dev mailing list >>>>> gatein-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/gatein-dev >>>>> >>>> >>> >> >> _______________________________________________ >> gatein-dev mailing list >> gatein-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/gatein-dev >> > > _______________________________________________ gatein-dev mailing list gatein-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/gatein-dev