Extract ForwardedSharingDetails to own collection #777

jkoenig134 · 2025-10-01T14:31:36Z

Readiness checklist

I added/updated tests.
I ensured that the PR title is good enough for the changelog.
I labeled the PR.
I self-reviewed the PR.

Description

…-to-own-collection

Milena-Czierlinski

Looks overall good to me, so feel free to continue.

I know the naming isn't final yet, just wanted to already mention that now that we also flattened the peerSharingDetails, I think we can simplify forwardedSharingDetails e.g. to only forwardedDetails/forwardingDetails. "forwarded" and "sharing" sounds somewhat doppelt-gemoppelt.

Milena-Czierlinski · 2025-10-15T14:14:40Z

...ges/consumption/src/modules/requests/itemProcessors/utility/createAppropriateResponseItem.ts

+        const deletionStatus = await attributesController.getForwardedSharingDetailsNotDeletedByRecipient(latestSharedVersion, requestInfo.peer);
+        if (deletionStatus) {


I noticed that here you refactored the use of hasDeletionStatusUnequalDeletedByRecipient while in other places you kept that method. Should we do it coherently?

Milena-Czierlinski · 2025-10-15T14:22:03Z

packages/consumption/src/modules/attributes/AttributesController.ts

-        await this.updateAttributeUnsafe(attribute);
+        await this.upsertForwardedSharingDetailsForPeer(attribute, peer, sharingDetails);
+
+        // TODO: this event is strange, why do you want to listen on that?


I thought that maybe we need to do some reloading in the app. If we don't see a reason for this event, I am fine with remving it.

britsta · 2025-10-17T11:25:27Z

packages/consumption/src/consumption/ConsumptionIds.ts

    public static readonly setting = new CoreIdHelper("LCLSET");

    public static readonly attribute = new CoreIdHelper("ATT");
+    public static readonly attributeForwardedSharingDetails = new CoreIdHelper("ATTFSD");


If we decided to change the naming "forwardedSharingDetails" to "forwardedDetails/forwardingDetails" as @Milena-Czierlinski suggested, we must remember changing "ATTFSD" to "ATTFD" as well.

good catch :)

britsta · 2025-10-17T11:51:51Z

packages/runtime/src/useCases/consumption/attributes/GetAttributes.ts

+        // TODO: get the number of forwards
+        // TODO: filter for query.numberOfForwards


Will this be done in this PR?

britsta · 2025-10-17T12:03:25Z

packages/consumption/test/modules/attributes/AttributesController.test.ts

            );
-            expect(updatedAttribute.forwardedSharingDetails).toHaveLength(1);
-            expect(updatedAttribute.forwardedSharingDetails![0].deletionInfo).toBeUndefined();
+            expect(updatedAttribute.numberOfForwards).toBe(1);


This forwardedSharingDetail should be fetched and checked if its deletionInfo is undefined.

britsta · 2025-10-17T12:05:01Z

packages/runtime/src/useCases/consumption/attributes/GetAttributes.ts

-    "forwardedSharingDetails.deletionInfo"?: string | string[];
-    "forwardedSharingDetails.deletionInfo.deletionStatus"?: string | string[];
-    "forwardedSharingDetails.deletionInfo.deletionDate"?: string | string[];
+    numberOfForwards?: string | string[];


Why not number | number[] (as the type of numberOfForwards of the LocalAttributeDTO is number)?

britsta · 2025-10-17T12:10:56Z

...Processors/attributeDeleted/ForwardedAttributeDeletedByPeerNotificationItemProcessor.test.ts

+        const updatedPredecessorForwardedSharingDetails = await consumptionController.attributes.getForwardedSharingDetailsForAttribute(databaseAttribute);
+        expect(updatedPredecessorForwardedSharingDetails[0].deletionInfo!.deletionStatus).toStrictEqual(EmittedAttributeDeletionStatus.DeletedByRecipient);


This test is not about Attribute succession.

The same applies to other variable names in this file.

Suggested change

const updatedPredecessorForwardedSharingDetails = await consumptionController.attributes.getForwardedSharingDetailsForAttribute(databaseAttribute);

expect(updatedPredecessorForwardedSharingDetails[0].deletionInfo!.deletionStatus).toStrictEqual(EmittedAttributeDeletionStatus.DeletedByRecipient);

const updatedAttributeForwardedSharingDetails = await consumptionController.attributes.getForwardedSharingDetailsForAttribute(databaseAttribute);

expect(updatedAttributeForwardedSharingDetails[0].deletionInfo!.deletionStatus).toStrictEqual(EmittedAttributeDeletionStatus.DeletedByRecipient);

britsta · 2025-10-17T12:20:31Z

packages/runtime/src/dataViews/DataViewExpander.ts

-    private expandForwardingPeers(localAttribute: OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute): string[] | undefined {
-        if (!localAttribute.forwardedSharingDetails || localAttribute.forwardedSharingDetails.length === 0) return;
+    private expandForwardingPeers(_localAttribute: OwnIdentityAttribute | OwnRelationshipAttribute | PeerRelationshipAttribute): string[] | undefined {
+        // TODO: enable when finding out how to proceed with this


Will this be done in this PR?

britsta · 2025-10-17T12:37:24Z

packages/consumption/src/modules/attributes/local/attributeTypes/LocalAttribute.ts

+    // this is not persisted, only used when returning attributes via the API
+    public numberOfForwards?: number;


This is the reason why numberOfForwards isn't added to ILocalAttribute and LocalAttributeJSON, right?

Would it be appropriate here to add validation that numberOfForwards cannot be negative?

Should numberOfForwards be added to the technicalProperties above?

britsta · 2025-10-17T12:39:20Z

packages/runtime/src/useCases/consumption/attributes/GetAttributes.ts

Schema.ts needs to be updated.

britsta · 2025-10-17T12:51:33Z

packages/consumption/src/modules/attributes/local/attributeTypes/LocalAttribute.ts

+    // this is not persisted, only used when returning attributes via the API
+    public numberOfForwards?: number;


I think this property must be removed here and add to OwnIdentityAttribute, OwnRelationshipAttribute and PeerRelationshipAttribute instead, as for the other LocalAttribute types this property should never be set. @Milena-Czierlinski What do you think?

britsta · 2025-10-17T12:52:28Z

packages/consumption/src/modules/attributes/local/attributeTypes/LocalAttribute.ts

    @serialize()
    public wasViewedAt?: CoreDate;

+    // this is not persisted, only used when returning attributes via the API


Suggested change

// this is not persisted, only used when returning attributes via the API

// this is not persisted, only used when returning Attributes via the API

chore: add attributeId to ForwardedSharingDetails

95f148f

jkoenig134 added the refactoring Refactoring of code label Oct 1, 2025

jkoenig134 marked this pull request as draft October 1, 2025 14:37

jkoenig134 and others added 9 commits October 2, 2025 10:41

chore: fix error for now

879be9d

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

e0f19ca

…-to-own-collection

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

612d2a8

…-to-own-collection

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

33af99d

…-to-own-collection

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

8491549

…-to-own-collection

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

c122bd8

…-to-own-collection

feat: update code

254fb27

test: start updateing consumption tests

48b3b0e

Merge branch 'rework-attributes' into extract-ForwardedSharingDetails…

0edcedb

…-to-own-collection

Milena-Czierlinski reviewed Oct 15, 2025

View reviewed changes

britsta reviewed Oct 17, 2025

View reviewed changes

refactor: ForwardedSharingDetails -> ForwardingDetails

95065d5

		const deletionStatus = await attributesController.getForwardedSharingDetailsNotDeletedByRecipient(latestSharedVersion, requestInfo.peer);
		if (deletionStatus) {

		// TODO: get the number of forwards
		// TODO: filter for query.numberOfForwards

		const updatedPredecessorForwardedSharingDetails = await consumptionController.attributes.getForwardedSharingDetailsForAttribute(databaseAttribute);
		expect(updatedPredecessorForwardedSharingDetails[0].deletionInfo!.deletionStatus).toStrictEqual(EmittedAttributeDeletionStatus.DeletedByRecipient);

		// this is not persisted, only used when returning attributes via the API
		public numberOfForwards?: number;

Extract ForwardedSharingDetails to own collection #777

Are you sure you want to change the base?

Extract ForwardedSharingDetails to own collection #777

Uh oh!

Conversation

jkoenig134 commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Readiness checklist

Description

Uh oh!

Milena-Czierlinski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jkoenig134 commented Oct 1, 2025 •

edited

Loading