Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Field Dependencies #152

Closed
seandenigris opened this issue Aug 4, 2020 · 6 comments
Closed

Field Dependencies #152

seandenigris opened this issue Aug 4, 2020 · 6 comments
Labels
enhancement stale Closed after 6 months to limit bug bankruptcy; can be reopened if still important

Comments

@seandenigris
Copy link
Member

Sometimes, changing a field e.g. in a Magritte form requires other fields to be updated. This is a problem because Magritte form models (e.g. components, Morphic adapters) don't deal with the actual model object, but a memento - a dumb dictionary standing in until all modifications are deemed valid. Thus, changing a field means updating a dictionary entry, not changing the actual object, and therefore not knowing anything about dependent fields that should also change. For a concrete example, take a car loan payment calculator - when the deposit is increased, the monthly payment should go down - and it might be needed for this to happen live in a web form.

Ramon Leon lamented this missing feature on his blog (see the comments) and it seemed to be one of the main reasons he abandoned Magritte.

How can this be solved? One way might be to add the capability to describe field dependencies (at least side-effect free ones) e.g. a utility method that takes a new value for field A and returns a compatible value for field B.

@eMaringolo
Copy link
Contributor

eMaringolo commented Sep 17, 2020

On a heavy use of descriptions in a fork of an old version of Magritte we didn't use Mementos, but instead we used something called BufferedModel that was used as the model of a Presenter (or Seaside component), so instead of working on the actual object, you work on a wrapper of it, that has the original model and a copy of it, the BufferedModel forwards all messages to the copy, so you're working with the actual object.

At the end if you accept the change, it applied the changes from the copy to the original model using the descriptions accessors to read and write. If you discard the change, you simply discard the object or reset the internal copy to a new copy for the model.

The side effect of this is that the validation is that you always have the real object, and so can use other descriptions to read the value you want to compare it to, or even better, to delegate the validation to the object itself.

I always saw the validations in the descriptions as something that validates at "input" level (empty, valid characters, mandatory selection, etc.) but not at "domain" level (e.g. whether the selection is meaningful, etc.).

@seandenigris
Copy link
Member Author

Interesting perspective. Thanks :) Do you have any code for the memento-alternative? Definitely interested...

@eMaringolo
Copy link
Contributor

I should dig in the code, but it was basically something like:

doesNotUnderstand: message 
	^message forwardTo: self subjectCopy

apply
	"Apply the changes from subjectCopy to subject."

	1 to: self subject class instSize do: [:idx | self subject instVarAt: idx put: (self subjectCopy instVarAt: idx)].

I don't remember how we used descriptions there or if the apply used a oneWayBecome: instead of a instVar copy. The issue with the become is that it can cause side-effects when the changes modifies the object hash and was in a hashed collection.

I'll check and get back, because I'm starting to use Magritte in Seaside and need these features as well.

@stale
Copy link

stale bot commented Nov 16, 2020

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

@stale stale bot added the stale Closed after 6 months to limit bug bankruptcy; can be reopened if still important label Nov 16, 2020
@stale stale bot closed this as completed Nov 23, 2020
@seandenigris
Copy link
Member Author

Here's an even more complex case: I have a project object which has a #stakeholders field described thusly:

stakeholdersDescription
	<magritteDescription>
	
	^ MAToManyRelationDescription new
		accessor: #stakeholders;
		priority: 200;
		default: Set new;
		classes: PpStakeholder;
		yourself

A stakeholder isn't an easy object to construct, as it takes a project, a person and a role.

One type of project generally has a certain kind of stakeholder i.e. one with a fixed role. Since the project is self, the only state we need is the person. Thus, there is this separate description:

waiteeDescription
	<magritteDescription>
	
	^ MAToOneRelationDescription new
		accessor: #waitee;
		priority: 200;
		classes: MpRelatableEntity allSubclasses;
		gtSearchSource: (MessageSend receiver: self selector: #addressBook); 
		yourself

#waitee: then does some magic:

waitee: aPerson

	self stakeholders 
		detect: [ :e | e role = #waitee ]
		ifFound: [ :e | e person: aPerson ]
		ifNone: [ (PpStakeholder project: self person: aPerson role: #waitee) link ]

Now for the problem: since there are two descriptions affecting the same data, #stakeholders, one can clobber the other. For example, the first caches an empty set. During the memento commit, the second adds a stakeholder, and the first overrides with an empty set.

The more I think about this and other problems, the more I feel that what gets cached should not be raw before and after values, but changes, which can then be checked for conflicts. So the waitee change would be a macro including the addition to stakeholders, and the stakeholders would be a no op (or you could resolve conflicts)

@seandenigris seandenigris reopened this Aug 22, 2022
@stale stale bot closed this as completed Sep 20, 2022
@seandenigris seandenigris pinned this issue Sep 20, 2022
@seandenigris seandenigris reopened this Sep 20, 2022
@stale stale bot removed the stale Closed after 6 months to limit bug bankruptcy; can be reopened if still important label Sep 20, 2022
@stale
Copy link

stale bot commented Nov 22, 2022

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

@stale stale bot added the stale Closed after 6 months to limit bug bankruptcy; can be reopened if still important label Nov 22, 2022
@stale stale bot closed this as completed Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement stale Closed after 6 months to limit bug bankruptcy; can be reopened if still important
Projects
None yet
Development

No branches or pull requests

2 participants