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

feat: [DHIS2-16922] Delete Tracked entity from profile Widget #3545

Merged
merged 17 commits into from
Mar 5, 2024

Conversation

simonadomnisoru
Copy link
Contributor

@simonadomnisoru simonadomnisoru commented Feb 28, 2024

DHIS2-16922

Tech summary

  • canCascadeDeleteTei - checks for the authority and caches the result using react query.
  • canWriteData - checks that the user has write access to the tracked entity type and the current program.
  • The WidgetProfile exposes the onDeleteSuccess callback that will be used to navigate to main page by the enrollment pages.
  • enable the forceUpdateOnMount logic for tracker WL. The WL will be updated when navigating to the home page so that the recently deleted tracked entity will no longer be displayed in the table.

@simonadomnisoru simonadomnisoru marked this pull request as ready for review March 1, 2024 17:42
@simonadomnisoru simonadomnisoru requested a review from a team as a code owner March 1, 2024 17:42
Copy link
Contributor

@eirikhaugstulen eirikhaugstulen left a comment

Choose a reason for hiding this comment

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

Hey @simonadomnisoru!
Some quick comments, but otherwise great job on this! 🎉

Comment on lines 10 to 42
return i18n.t("You don't have access to delete this {{TETName}}", {
TETName: trackedEntityTypeName,
interpolation: { escapeValue: false },
});
}
if (!canCascadeDeleteTei) {
return i18n.t("You can't delete this {{TETName}} when it has associated enrollments and events", {
TETName: trackedEntityTypeName,
interpolation: { escapeValue: false },
});
}
return '';
};

export const DeleteMenuItem = ({
trackedEntityTypeName,
canCascadeDeleteTei,
canWriteData,
setActionsIsOpen,
setDeleteModalIsOpen,
}: Props) => {
const tooltipContent = getTooltipContent(canWriteData, canCascadeDeleteTei, trackedEntityTypeName);

return (
<ConditionalTooltip content={tooltipContent} enabled={!canWriteData || !canCascadeDeleteTei}>
<MenuItem
destructive
dense
icon={<IconDelete16 />}
label={i18n.t('Delete {{TETName}}', {
TETName: trackedEntityTypeName,
interpolation: { escapeValue: false },
})}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to use a lowercased trackedEntityTypeName, similar to what we use other places?

<Button onClick={() => setOpenModal(false)} secondary>
{i18n.t('No, cancel')}
</Button>
<Button onClick={() => deleteMutation(trackedEntity)} primary loading={deleteLoading}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this button be destructive?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we still keep this file after the discussion we had in Changelogs-PR, or should we use the one in Buttons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I move it into the Buttons folder in the latest commit. Thanks!

@simonadomnisoru
Copy link
Contributor Author

Hey @simonadomnisoru! Some quick comments, but otherwise great job on this! 🎉

Thanks for the feedback @eirikhaugstulen. I implemented your suggestions. Can you have a look?

@simonadomnisoru simonadomnisoru requested review from a team as code owners March 5, 2024 07:21
authorities &&
authorities.some(authority => authority === auth.ALL || authority === auth.F_TEI_CASCADE_DELETE),
};
const { data } = useApiMetadataQuery<any>(queryKey, queryFn, queryOptions);
Copy link
Member

Choose a reason for hiding this comment

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

How to properly use generics here is probably something we can figure out later @eirikhaugstulen @simonadomnisoru

Copy link
Member

@JoakimSM JoakimSM Mar 5, 2024

Choose a reason for hiding this comment

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

I assume this is the same as src/core_modules/capture-core/components/Tooltips/ConditionalTooltip/ConditionalTooltip.component.js. Let's use that one even though we are in a "self-contained" Widget.

Comment on lines 9 to 20
if (!canWriteData) {
return i18n.t("You don't have access to delete this {{trackedEntityTypeName}}", {
trackedEntityTypeName,
interpolation: { escapeValue: false },
});
}
if (!canCascadeDeleteTei) {
return i18n.t("You can't delete this {{trackedEntityTypeName}} when it has associated enrollments and events", {
trackedEntityTypeName,
interpolation: { escapeValue: false },
});
}
Copy link
Member

Choose a reason for hiding this comment

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

There has been some last minute changes to the design here. Let's use the tooltip content: "You do not have access to delete this $TET" (for both the cases here)

Copy link
Member

@JoakimSM JoakimSM left a comment

Choose a reason for hiding this comment

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

🎉

Copy link

github-actions bot commented Mar 5, 2024

@@ -1029,6 +1029,10 @@ Click the **Edit** button to make changes to the tracked entity instance profile

![](resources/images/enrollment-dash-tei-profile-widget-edit.png)

Click the **Delete ${tracked entity type}** button to delete the tracked entity. You can confirm the action from the dialog. Once confirmed, tracked entity and all its associated enrollment and events across all programs will be deleted.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Click the **Delete ${tracked entity type}** button to delete the tracked entity. You can confirm the action from the dialog. Once confirmed, tracked entity and all its associated enrollment and events across all programs will be deleted.
Click the **Delete ${tracked entity type}** button to delete the tracked entity. You can confirm the action from the dialog. Once confirmed, tracked entity and all its associated enrollment and events across all programs will be deleted. To delete a ${tracked entity type} that has any enrollments, the user needs the authority **Delete tracked entity instance and associated enrollments and events**.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion. It's implemented in the latest commit. Thanks for the feedback!

Copy link

@geethaalwan geethaalwan left a comment

Choose a reason for hiding this comment

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

Tested successfully on 2.41 ,2.40.4,2.39.6,2.38.7 versions

@simonadomnisoru simonadomnisoru merged commit fb8171e into master Mar 5, 2024
37 of 38 checks passed
@simonadomnisoru simonadomnisoru deleted the DHIS2-16922 branch March 5, 2024 16:01
dhis2-bot added a commit that referenced this pull request Mar 5, 2024
# [100.63.0](v100.62.0...v100.63.0) (2024-03-05)

### Features

* [DHIS2-16922] Delete Tracked entity from profile Widget ([#3545](#3545)) ([fb8171e](fb8171e))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.63.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

6 participants