Add actions field to Card. Handle PROPAGATE_READ_ACK_TO_PARENT_CARD action (#5461) #5651
Conversation
quinarygio
changed the title
quinarygio
force-pushed
the
FE-5461
branch
3 times, most recently
from
2ba1344
to
1b4c9f3
Compare
| if (hasBeenRead && children) { | ||
| for (let child of children) { | ||
| if (!child.hasBeenRead) { | ||
| hasBeenRead = false; | ||
| break; | ||
| } |
There was a problem hiding this comment.
This shall take into account the PROPAGATE_READ_ACK_TO_PARENT_CARD
| const parent = this.lightCards.get(card.parentCardId); | ||
| if (card.actions?.includes(CardAction.PROPAGATE_READ_ACK_TO_PARENT_CARD)) { | ||
| if (parent.hasBeenRead && !card.hasBeenRead) { | ||
| CardService.deleteUserCardRead(parent.uid).subscribe(); |
There was a problem hiding this comment.
You do not need to propagate the read status of the parent to the back , the read status is always processed on the front bases on the read status of the child .
| this.setLightCardRead(parent.id, false); | ||
| } | ||
| if (parent.hasBeenAcknowledged && !card.hasBeenAcknowledged) { | ||
| AcknowledgeService.deleteUserAcknowledgement(parent.uid).subscribe(); |
There was a problem hiding this comment.
You do not need to propagate the ack status of the parent to the back , the ack status is always processed on the front bases on the ack status of the child . So you need to modify ui/main/src/app/modules/card/components/card-ack/card-ack.component.ts to take into account the ack of the child cards (method setAcknowledgeButtonVisibility) in the card detail view
| const childCards = OpfabStore.getLightCardStore().getChildCards(this.card.id); | ||
| if (childCards) { | ||
| childCards.forEach(child => { | ||
| if (child.actions?.includes(CardAction.PROPAGATE_READ_ACK_TO_PARENT_CARD)) { | ||
| AcknowledgeService.deleteUserAcknowledgement(child.uid).subscribe(); | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
I think it is not necessary to cancel ack for child cards as if the parent ack is cancelled the card will be considered as unack whatever the status of the childs
| @@ -246,6 +263,18 @@ export class LightCardsStore { | |||
| } else { | |||
| this.childCards.set(card.parentCardId, [card]); | |||
| } | |||
|
|
|||
| const parent = this.lightCards.get(card.parentCardId); | |||
There was a problem hiding this comment.
parent could be null if child is received before parent cards (we have no guarantee of the receive order when loading cards) so you should check this case
But if it arise , the processing of the ack/read should be done when receiving the card , it is already done for read card in your code line 239 but it has to be done as well for ack
quinarygio
force-pushed
the
FE-5461
branch
2 times, most recently
from
585cc55
to
8414a2b
Compare
quinarygio
force-pushed
the
FE-5461
branch
2 times, most recently
from
777fd5f
to
8b6e97c
Compare
| const childIndex = children.findIndex(child => child.id === card.id); | ||
| if (childIndex >= 0) children.splice(childIndex); |
There was a problem hiding this comment.
I don't understand why this two lines of code ? could you explain ?
There was a problem hiding this comment.
I saw that when a child card is updated we end up having the both version of the child card in the "children" collection. So before adding a child card I check if it already exists and eventually remove it before adding the updated one.
| } | ||
| } | ||
|
|
||
| private setReadAndAckOnParentCardIfNeeded(card: LightCard) { |
There was a problem hiding this comment.
I would suggest naming it unreadAndUnackParentCardIfNeeded
| .subscribe(); | ||
| } | ||
|
|
||
| private updateAcknowledgeButtonVisibilityIfNeeded(childCard) { |
There was a problem hiding this comment.
i would suggest naming it " updateAcknowledgeButtonVisibilityIfCardsIsChildOfCurrentCard"
| } | ||
| this.lastCardSetToReadButNotYetOnFeed = this.card; | ||
| CardService.postUserCardRead(this.card.uid).subscribe(); | ||
|
|
||
| } else this.updateLastReadCardStatusOnFeedIfNeeded(); | ||
|
|
||
| if (this.childCards) { | ||
| this.childCards.forEach(child => { | ||
| if (child.actions?.includes(CardAction.PROPAGATE_READ_ACK_TO_PARENT_CARD) && !child.hasBeenRead) { | ||
| CardService.postUserCardRead(child.uid).subscribe(); | ||
| } | ||
| }) | ||
| } | ||
| this.lastCardSetToReadButNotYetOnFeed = this.card; | ||
|
|
||
| } |
There was a problem hiding this comment.
this.lastCardSetToReadButNotYetOnFeed = this.card shall only be done if read status change
So keep the old this.lastCardSetToReadButNotYetOnFeed = this.card;
and add a new after CardService.postUserCardRead(child.uid).subscribe();
| private computeCardHasBeenAcknowledged() { | ||
| this.card.hasBeenAcknowledged = OpfabStore.getLightCardStore().isLightCardHasBeenAcknowledged(this.card); | ||
| } | ||
|
|
There was a problem hiding this comment.
the ack status is not used directly in card-body.component , i suggest to avoid dealing with ack here but do the processing where it is needed i.e in setAcknowledgeButtonVisibility method in card-ack.component.ts
| "lttd" : ${current_date_in_milliseconds_from_epoch_plus_3minutes}, | ||
| "lttd" : ${current_date_in_milliseconds_from_epoch_plus_4hours}, | ||
| "timeSpans" : [ | ||
| {"start" : ${current_date_in_milliseconds_from_epoch_plus_4hours}} | ||
| ] | ||
| ], | ||
| "actions" : ["PROPAGATE_READ_ACK_TO_PARENT_CARD"] |
There was a problem hiding this comment.
I do not think these modifications are useful ?
…ction (#5461) Signed-off-by: Giovanni Ferrari <giovanni.ferrari@soft.it>
|
Fix #5461