[
https://issues.apache.org/jira/browse/ISIS-941?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dan Haywood updated ISIS-941:
-----------------------------
Description:
This broke in 1.6.0 because of https://issues.apache.org/jira/browse/ISIS-837
https://github.com/apache/isis/blob/0af3219129076161143bd388e3fc3ec039cbed79/component/viewer/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L582
There are two conflicting requirements. In some cases we want a derived
property to appear to be editable... so that it udpates other stuff. In other
cases the derived property is not editable, so we should do nothing.
In
https://github.com/apache/isis/blob/1b10065e7fefc591e7e510ecb7a5ea6dba17f485/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/update/PropertySetAndClearFacetFactory.java#L78
we go looking for setters and clears. But if there is no setter, we install a
NotPersistedFacetInferred.
The stuff about the NotPersistedFacet etc is, in fact, irrelevant. We do still
need this facet, but it's all to do with persistence and transactions. For
example, "isNotPersisted" on OneToOneAssociation in several places (eg
IsisTransaction#enlist..., and PersistedAlgorithmDefault) in order to ensure we
only deal with properties that have "backing" data.
And that means the fix in ISIS-837 is correct so far as it goes...
EntityModel#apply shouldn't pay any attentino to the NotPersistedFacet, because
as the comment from ISIS-837 and the first use case says, in these cases we DO
want to flush the changes into the Isis runtime.
However, ISIS-837 didn't go far enough .... EntityModel#apply must also account
for the property being disabled and in these cases do nothing. Note that the
DisabledFacet is added to the OneToOneAssociation by virtue of
DisabledFacetOnPropertyInferredFacetFactory. (See also the comment that is in
PropertySetAndClearFacetFactory that shows that the facet used to be installed
there, but was moved for other reasons).
was:
This broke in 1.6.0 because of https://issues.apache.org/jira/browse/ISIS-837
https://github.com/apache/isis/blob/0af3219129076161143bd388e3fc3ec039cbed79/component/viewer/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L582
There are two conflicting requirements. In some cases we want a derived
property to appear to be editable... so that it udpates other stuff. In other
cases the derived property is not editable, so we should do nothing.
In
https://github.com/apache/isis/blob/1b10065e7fefc591e7e510ecb7a5ea6dba17f485/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/update/PropertySetAndClearFacetFactory.java#L78
we install a NotPersistedFacetInferred if there is no setter/clear.
That is correct... we use "isNotPersisted" on OneToOneAssociation in several
places (eg IsisTransaction#enlist..., and PersistedAlgorithmDefault) in order
to ensure we only deal with properties that have "backing" data.
The fix in ISIS-837 is correct so far as it goes It's not enough
EntityModel#apply to go looking for the NotPersistedFacet, because as the
comment from ISIS-837 and the first use case says, in these cases we DO want to
flush the changes into the Isis runtime.
However, EntityModel#apply must also account for the property being disabled
(which is inferred from it not having any setter/clear facets, see
DisabledFacetOnPropertyInferredFacetFactory).
> Wicket viewer shouldn't try to flush properties that are disabled.
> ------------------------------------------------------------------
>
> Key: ISIS-941
> URL: https://issues.apache.org/jira/browse/ISIS-941
> Project: Isis
> Issue Type: Bug
> Components: Viewer: Wicket
> Affects Versions: viewer-wicket-1.6.0
> Reporter: Dan Haywood
> Assignee: Dan Haywood
> Priority: Minor
> Fix For: viewer-wicket-1.8.0
>
>
> This broke in 1.6.0 because of https://issues.apache.org/jira/browse/ISIS-837
> https://github.com/apache/isis/blob/0af3219129076161143bd388e3fc3ec039cbed79/component/viewer/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L582
> There are two conflicting requirements. In some cases we want a derived
> property to appear to be editable... so that it udpates other stuff. In
> other cases the derived property is not editable, so we should do nothing.
> In
> https://github.com/apache/isis/blob/1b10065e7fefc591e7e510ecb7a5ea6dba17f485/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/update/PropertySetAndClearFacetFactory.java#L78
> we go looking for setters and clears. But if there is no setter, we install
> a NotPersistedFacetInferred.
> The stuff about the NotPersistedFacet etc is, in fact, irrelevant. We do
> still need this facet, but it's all to do with persistence and transactions.
> For example, "isNotPersisted" on OneToOneAssociation in several places (eg
> IsisTransaction#enlist..., and PersistedAlgorithmDefault) in order to ensure
> we only deal with properties that have "backing" data.
> And that means the fix in ISIS-837 is correct so far as it goes...
> EntityModel#apply shouldn't pay any attentino to the NotPersistedFacet,
> because as the comment from ISIS-837 and the first use case says, in these
> cases we DO want to flush the changes into the Isis runtime.
> However, ISIS-837 didn't go far enough .... EntityModel#apply must also
> account for the property being disabled and in these cases do nothing. Note
> that the DisabledFacet is added to the OneToOneAssociation by virtue of
> DisabledFacetOnPropertyInferredFacetFactory. (See also the comment that is
> in PropertySetAndClearFacetFactory that shows that the facet used to be
> installed there, but was moved for other reasons).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
