Skip to content

LT-22448: Implement ItemDataModified in Pub/Sub system#741

Open
mark-sil wants to merge 2 commits intomainfrom
LT-22448
Open

LT-22448: Implement ItemDataModified in Pub/Sub system#741
mark-sil wants to merge 2 commits intomainfrom
LT-22448

Conversation

@mark-sil
Copy link
Contributor

@mark-sil mark-sil commented Mar 5, 2026

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Mar 5, 2026

NUnit Tests

    1 files  ±0      1 suites  ±0   5m 49s ⏱️ -4s
4 407 tests ±0  4 320 ✅ ±0  87 💤 ±0  0 ❌ ±0 
4 416 runs  ±0  4 329 ✅ ±0  87 💤 ±0  0 ❌ ±0 

Results for commit 2b410af. ± Comparison against base commit bcfbea0.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasonleenaylor reviewed 2 files and made 1 comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on mark-sil).


Src/xWorks/RecordClerk.cs line 312 at r1 (raw file):

				if (decorator == null)
					break;
				da = decorator.BaseSda;

I think the RecordClerk is not the class that should actually be subscribing. The decorator that implements OnItemDataModified should actually be the one subscribed eliminating all this ugliness. May as well do that.

Code quote:

			var da = m_list.VirtualListPublisher;
			while (da != null)
			{
				if (da.GetType().GetMethod("OnItemDataModified") != null)
					ReflectionHelper.CallMethod(da, "OnItemDataModified", new [] {argument});
				var decorator = da as DomainDataByFlidDecoratorBase;
				if (decorator == null)
					break;
				da = decorator.BaseSda;

Copy link
Contributor Author

@mark-sil mark-sil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-sil made 1 comment.
Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on jasonleenaylor).


Src/xWorks/RecordClerk.cs line 312 at r1 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I think the RecordClerk is not the class that should actually be subscribing. The decorator that implements OnItemDataModified should actually be the one subscribed eliminating all this ugliness. May as well do that.

I agree, that is a much cleaner solution. I moved the subscribing to the ConcDecorator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants