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

Deleting a SharedAttribute does not validate if it's possible to send the Notification #398

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4473bf6
feat: revoke relationship in status pending when the neded attribute …
sebbi08 Jan 27, 2025
4f8d26e
chore: remove focused test
sebbi08 Jan 27, 2025
ef7653b
Merge branch 'main' into feature/ABL-354
sebbi08 Jan 27, 2025
5e2237a
feat: add error message when deleting an attribute while a relationsh…
sebbi08 Jan 31, 2025
d76fbce
Merge branch 'main' into feature/ABL-354
mergify[bot] Jan 31, 2025
77cb61f
revert: unwanted change
sebbi08 Jan 31, 2025
4e6814f
chore: pr comments
sebbi08 Jan 31, 2025
c5994b1
Merge branch 'main' into feature/ABL-354
mergify[bot] Feb 3, 2025
c9f4062
Merge branch 'main' into feature/ABL-354
mergify[bot] Feb 3, 2025
f670e7e
chore: pr comments
sebbi08 Feb 4, 2025
cd7fcdc
Update packages/runtime/test/consumption/attributes.test.ts
sebbi08 Feb 4, 2025
af5fc20
Update packages/runtime/test/lib/testUtils.ts
sebbi08 Feb 4, 2025
11f0270
Update packages/runtime/test/consumption/attributes.test.ts
sebbi08 Feb 4, 2025
3724471
chore: fix renaming
sebbi08 Feb 4, 2025
e76a5f1
chore: fix renaming
sebbi08 Feb 4, 2025
e470075
chore: fix sending messsage when relationsip is not activatable anymore
sebbi08 Feb 4, 2025
356abea
Merge branch 'main' into feature/ABL-354
mergify[bot] Feb 4, 2025
96d653a
chore: improve return type and variable naming
sebbi08 Feb 4, 2025
054e1d2
chore: pr comments
sebbi08 Feb 5, 2025
02f5178
Merge branch 'main' into feature/ABL-354
mergify[bot] Feb 5, 2025
d9a8294
Merge branch 'main' into feature/ABL-354
mergify[bot] Feb 5, 2025
d8dce26
Update packages/transport/src/modules/messages/MessageController.ts
sebbi08 Feb 6, 2025
4eb668e
chore: make notificationId optional
sebbi08 Feb 7, 2025
a26e5fd
chore: make notificationId optional
sebbi08 Feb 7, 2025
03aaebd
chore: fix tests
sebbi08 Feb 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/runtime/src/useCases/common/RuntimeErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class Attributes {
public cannotDeleteSharedAttributeWhileRelationshipIsPending(): ApplicationError {
return new ApplicationError(
"error.runtime.attributes.cannotDeleteSharedAttributeWhileRelationshipIsPending",
"The shared Attribute cannot be deleted while the Relationship to the Peer is in status Pending."
"The shared Attribute cannot be deleted while the Relationship to the peer is in status 'Pending'."
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Notification, OwnSharedAttributeDeletedByOwnerNotificationItem } from "
import { CoreId } from "@nmshd/core-types";
import { AccountController, MessageController, RelationshipsController } from "@nmshd/transport";
import { Inject } from "@nmshd/typescript-ioc";
import { RelationshipStatus } from "../../../types";
import { AttributeIdString, NotificationIdString, RuntimeErrors, SchemaRepository, SchemaValidator, UseCase } from "../../common";

export interface DeleteOwnSharedAttributeAndNotifyPeerRequest {
Expand Down Expand Up @@ -41,9 +40,9 @@ export class DeleteOwnSharedAttributeAndNotifyPeerUseCase extends UseCase<Delete
return Result.fail(RuntimeErrors.attributes.isNotOwnSharedAttribute(ownSharedAttributeId));
}

const relationship = await this.relationshipsController.getRelationshipToIdentity(ownSharedAttribute.shareInfo.peer);
const canSendMessageResult = await this.relationshipsController.canSendMessage(ownSharedAttribute.shareInfo.peer);

if (relationship && relationship.status === RelationshipStatus.Pending) {
if (canSendMessageResult.isError) {
return Result.fail(RuntimeErrors.attributes.cannotDeleteSharedAttributeWhileRelationshipIsPending());
sebbi08 marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Notification, PeerSharedAttributeDeletedByPeerNotificationItem } from "
import { CoreId } from "@nmshd/core-types";
import { AccountController, MessageController, RelationshipsController } from "@nmshd/transport";
import { Inject } from "@nmshd/typescript-ioc";
import { RelationshipStatus } from "../../../types";
import { AttributeIdString, NotificationIdString, RuntimeErrors, SchemaRepository, SchemaValidator, UseCase } from "../../common";

export interface DeletePeerSharedAttributeAndNotifyOwnerRequest {
Expand Down Expand Up @@ -40,13 +39,15 @@ export class DeletePeerSharedAttributeAndNotifyOwnerUseCase extends UseCase<Dele
if (!peerSharedAttribute.isPeerSharedAttribute(peerSharedAttribute.shareInfo?.peer)) {
return Result.fail(RuntimeErrors.attributes.isNotPeerSharedAttribute(peerSharedAttributeId));
}
const relationship = await this.relationshipsController.getRelationshipToIdentity(peerSharedAttribute.shareInfo.peer);

if (relationship && relationship.status === RelationshipStatus.Pending) {
const canSendMessageResult = await this.relationshipsController.canSendMessage(peerSharedAttribute.shareInfo.peer);

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

if (canSendMessageResult.isError) {
return Result.fail(RuntimeErrors.attributes.cannotDeleteSharedAttributeWhileRelationshipIsPending());
}

const validationResult = await this.attributesController.validateFullAttributeDeletionProcess(peerSharedAttribute);

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

if (validationResult.isError()) {
return Result.fail(validationResult.error);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Notification, ThirdPartyRelationshipAttributeDeletedByPeerNotificationI
import { CoreId } from "@nmshd/core-types";
import { AccountController, MessageController, RelationshipsController } from "@nmshd/transport";
import { Inject } from "@nmshd/typescript-ioc";
import { RelationshipStatus } from "../../../types";
import { AttributeIdString, NotificationIdString, RuntimeErrors, SchemaRepository, SchemaValidator, UseCase } from "../../common";

export interface DeleteThirdPartyRelationshipAttributeAndNotifyPeerRequest {
Expand Down Expand Up @@ -46,9 +45,9 @@ export class DeleteThirdPartyRelationshipAttributeAndNotifyPeerUseCase extends U
return Result.fail(RuntimeErrors.attributes.isNotThirdPartyRelationshipAttribute(thirdPartyRelationshipAttributeId));
}

const relationship = await this.relationshipsController.getRelationshipToIdentity(thirdPartyRelationshipAttribute.shareInfo.peer);
const canSendMessageResult = await this.relationshipsController.canSendMessage(thirdPartyRelationshipAttribute.shareInfo.peer);

if (relationship && relationship.status === RelationshipStatus.Pending) {
if (canSendMessageResult.isError) {
return Result.fail(RuntimeErrors.attributes.cannotDeleteSharedAttributeWhileRelationshipIsPending());
}

Expand Down
27 changes: 12 additions & 15 deletions packages/runtime/test/consumption/attributes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2690,9 +2690,8 @@ describe("DeleteAttributeUseCases", () => {

const content: CreateOwnRelationshipTemplateRequest["content"] = {
"@type": "RelationshipTemplateContent",
title: "Connect to HCM",
title: "aTitle",
onNewRelationship: {
title: "Connect to HCM2",
items: [item],
"@type": "Request"
}
Expand All @@ -2715,7 +2714,7 @@ describe("DeleteAttributeUseCases", () => {
content: {
value: {
"@type": "GivenName",
value: "123qwe"
value: "aGivenName"
}
}
});
Expand All @@ -2728,7 +2727,7 @@ describe("DeleteAttributeUseCases", () => {
}
});

const acceptRequest = await services2.consumption.incomingRequests.accept({
const acceptedRequest = await services2.consumption.incomingRequests.accept({
requestId: requestsForRelationship.value[0].id,
items: [
{
Expand All @@ -2738,7 +2737,7 @@ describe("DeleteAttributeUseCases", () => {
]
});

expect(acceptRequest).toBeSuccessful();
expect(acceptedRequest).toBeSuccessful();

const relationships = await syncUntilHasRelationships(services1.transport);
expect(relationships).toHaveLength(1);
Expand Down Expand Up @@ -2872,7 +2871,7 @@ describe("DeleteAttributeUseCases", () => {
expect(CoreDate.from(updatedPredecessor.deletionInfo!.deletionDate).isBetween(timeBeforeUpdate, timeAfterUpdate.add(1))).toBe(true);
});

test("should throw an error when deleting an OwnSharedAttribute when the Relationship is in status Pending", async () => {
test("should throw an error when deleting an PeerSharedAttribute when the Relationship is in status Pending", async () => {
const [services1, services2] = await runtimeServiceProvider.launch(2, {
enableRequestModule: true,
enableDeciderModule: true,
Expand All @@ -2883,7 +2882,7 @@ describe("DeleteAttributeUseCases", () => {
content: {
value: {
"@type": "GivenName",
value: "123qwe"
value: "aGivenName"
}
}
});
Expand All @@ -2897,9 +2896,8 @@ describe("DeleteAttributeUseCases", () => {

const content: CreateOwnRelationshipTemplateRequest["content"] = {
"@type": "RelationshipTemplateContent",
title: "Connect to HCM",
title: "aTitle",
onNewRelationship: {
title: "Connect to HCM2",
items: [item],
"@type": "Request"
}
Expand All @@ -2926,7 +2924,7 @@ describe("DeleteAttributeUseCases", () => {
}
});

const acceptRequest = await services2.consumption.incomingRequests.accept({
const acceptedRequest = await services2.consumption.incomingRequests.accept({
requestId: requestsForRelationship.value[0].id,
items: [
{
Expand All @@ -2935,7 +2933,7 @@ describe("DeleteAttributeUseCases", () => {
]
});

expect(acceptRequest).toBeSuccessful();
expect(acceptedRequest).toBeSuccessful();

const relationships = await syncUntilHasRelationships(services1.transport);
expect(relationships).toHaveLength(1);
Expand Down Expand Up @@ -3099,9 +3097,8 @@ describe("DeleteAttributeUseCases", () => {

const content: CreateOwnRelationshipTemplateRequest["content"] = {
"@type": "RelationshipTemplateContent",
title: "Connect to HCM",
title: "aTitle",
onNewRelationship: {
title: "Connect to HCM2",
items: [item],
"@type": "Request"
}
Expand All @@ -3126,7 +3123,7 @@ describe("DeleteAttributeUseCases", () => {
}
});

const acceptRequest = await services3.consumption.incomingRequests.accept({
const acceptedRequest = await services3.consumption.incomingRequests.accept({
requestId: requestsForRelationship.value[0].id,
items: [
{
Expand All @@ -3135,7 +3132,7 @@ describe("DeleteAttributeUseCases", () => {
]
});

expect(acceptRequest).toBeSuccessful();
expect(acceptedRequest).toBeSuccessful();

const relationships = await syncUntilHasRelationships(services2.transport);
expect(relationships).toHaveLength(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,4 +655,13 @@ export class RelationshipsController extends TransportController {

return relationship;
}

public async canSendMessage(recipient: CoreAddress): Promise<Result<void, CoreError>> {
const relationship = await this.getRelationshipToIdentity(recipient, RelationshipStatus.Pending);
sebbi08 marked this conversation as resolved.
Show resolved Hide resolved
if (relationship) {
Result.fail(TransportCoreErrors.messages.hasNeitherActiveNorTerminatedRelationship([recipient.address]));
}

return Result.ok(undefined);
}
sebbi08 marked this conversation as resolved.
Show resolved Hide resolved
}
Loading