[GitHub] [isis] danhaywood commented on a change in pull request #587: ISIS-2723: simplify facet precedence logic
danhaywood commented on a change in pull request #587:
URL: https://github.com/apache/isis/pull/587#discussion_r647538659
##
File path:
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/notinservicemenu/derived/NotInServiceMenuFacetDerivedFromDomainServiceFacet.java
##
@@ -34,7 +34,7 @@
public NotInServiceMenuFacetDerivedFromDomainServiceFacet(
final NatureOfService natureOfService, final FacetHolder holder) {
-super(holder);
+super(holder, Precedence.DERIVED);
Review comment:
Probably best to rename the facet here also, get rid of the word
"Derived" and replace with something like "Inferred".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
[GitHub] [isis] danhaywood commented on a change in pull request #587: ISIS-2723: simplify facet precedence logic
danhaywood commented on a change in pull request #587:
URL: https://github.com/apache/isis/pull/587#discussion_r647537605
##
File path:
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/hidden/HiddenTypeFacetDerivedFromAuthorization.java
##
@@ -29,14 +29,16 @@
import lombok.val;
-public class HiddenTypeFacetDerivedFromAuthorization extends FacetAbstract
implements HiddenTypeFacet {
+public class HiddenTypeFacetDerivedFromAuthorization
+extends FacetAbstract
+implements HiddenTypeFacet {
public static Class type() {
return HiddenTypeFacet.class;
}
public HiddenTypeFacetDerivedFromAuthorization(final FacetHolder holder) {
-super(type(), holder);
+super(type(), holder, Precedence.DERIVED);
Review comment:
Sure. Some thoughts (probably obvious) on the other precedences:
* FALLBACK is lowest priority if there are no other facets for that type, eg
infer the NamedFacet from the method name if not specified explicitly using an
annotation or layout
* the notion of "alwaysReplace" I can't remember, but I suspect it is very
similar to fallback.
* the notion of an underlying facet is where a higher precedence facet can
replace another facet, but keep hold of the underlying facet. In some cases
the replacing facet might wrap but delegate to the underlying, eg to translate
the name facet. If the replacing facet doesn't delegate to the underlying,
then there's no real need to retain that reference, but (I wouldn't be
surprised) if FacetUtil.addFacet(...) always just chains them anyway regardless.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
[GitHub] [isis] danhaywood commented on a change in pull request #587: ISIS-2723: simplify facet precedence logic
danhaywood commented on a change in pull request #587:
URL: https://github.com/apache/isis/pull/587#discussion_r647528913
##
File path:
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/actions/notinservicemenu/derived/NotInServiceMenuFacetDerivedFromDomainServiceFacet.java
##
@@ -34,7 +34,7 @@
public NotInServiceMenuFacetDerivedFromDomainServiceFacet(
final NatureOfService natureOfService, final FacetHolder holder) {
-super(holder);
+super(holder, Precedence.DERIVED);
Review comment:
as per comment below, not sure this is Derived, because it can never be
overridden by something more specific.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
[GitHub] [isis] danhaywood commented on a change in pull request #587: ISIS-2723: simplify facet precedence logic
danhaywood commented on a change in pull request #587:
URL: https://github.com/apache/isis/pull/587#discussion_r647528049
##
File path:
core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/hidden/HiddenTypeFacetDerivedFromAuthorization.java
##
@@ -29,14 +29,16 @@
import lombok.val;
-public class HiddenTypeFacetDerivedFromAuthorization extends FacetAbstract
implements HiddenTypeFacet {
+public class HiddenTypeFacetDerivedFromAuthorization
+extends FacetAbstract
+implements HiddenTypeFacet {
public static Class type() {
return HiddenTypeFacet.class;
}
public HiddenTypeFacetDerivedFromAuthorization(final FacetHolder holder) {
-super(type(), holder);
+super(type(), holder, Precedence.DERIVED);
Review comment:
Although this facet's class name has the word "derived" in it, I'm not
sure it automatically means that it's precedence is DERIVED.
The semantics of DERIVED (nb: this is not documented anywhere) is for a
facet that is inferred from other facets, but which could be overridden by
something that is more specific.
That isn't the case here: the type should always be hidden if all of the
members are.
It should probably be named
"HiddenTypeFacetInferredFromNoAuthorisationToAnyMember" or something similar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
