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-18017] Ability to unlink event from edit/view event page #3846

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

henrikmv
Copy link
Contributor

@henrikmv henrikmv commented Oct 14, 2024

DHIS2-18017:

  • Removed export from ScheduleInOrgUnitPlain.

  • Added UnlinkModal to handle unlinking relationships without deleting them.

  • Added UnlinkAndDeleteModal component to handle the unlinking and deletion of events.

  • AddedOverflowMenu.component.js with flyout menu with options to view, unlink, and delete events.

  • Added functionality to refresh data using the setUpdateData state.

@henrikmv henrikmv marked this pull request as ready for review October 21, 2024 07:26
@henrikmv henrikmv requested a review from a team as a code owner October 21, 2024 07:26
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 @henrikmv - this looks like a great start! As always, when we're doing quick actions on DHIS2 data, the complexity is always bigger that what we believe at first, but have a look at my comments and let's go through it if needed 🙌 Good work.

{i18n.t('Unlink relationship')}
</ModalTitle>
<ModalContent>
<p>{i18n.t('Are you sure you want to unlink this relationship?')}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

@cooper-joe @karolinelien

Do you think we should add some explaining texts to describe what we're doing here? If so, any suggestions. The user is removing the relationship between two linked events, but I'm afraid that Unlink this relationship can be a bit too technical?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we say "Are you sure you want to delete the relationship (or link?) between these two events?" I might need to remind myself of the wording in the modal otherwise, I know we want to avoid DHIS2 terminology if we can but I think we have succumbed to needing to use "events" in this functionality? :D

And maybe we want to add a note about "This will only delete the relationship, not the other event"?

Comment on lines +57 to +75
<MenuItem
label={i18n.t('Unlink event')}
icon={<IconLink16 />}
dataTest={'event-overflow-unlink-event'}
onClick={() => {
setUnlinkModalIsOpen(true);
setActionsIsOpen(false);
}}
/>
<MenuItem
label={i18n.t('Unlink and delete event')}
icon={<IconDelete16 />}
dataTest={'event-overflow-unlink-and-delete-event'}
destructive
onClick={() => {
setUnlinkAndDeleteModalIsOpen(true);
setActionsIsOpen(false);
}}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to do some validation here I think. The user has to have write access to the relationship type to do both actions. Additionally, the user has to have write access to the stage the event is in, and the organisation unit has to be within the user's capture scope

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.

3 participants