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-18018][DHIS2-17853] show related stages widget in view event page #3916

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

Conversation

simonadomnisoru
Copy link
Contributor

@simonadomnisoru simonadomnisoru commented Dec 19, 2024

DHIS2-18018

Tech summary:

  • Make the WidgetRelatedStages configurable in the enrollmentEventEditLayout page layout
  • Based on the user selected option show an action button in WidgetRelatedStages
  • use react-query to create the linked event and the relationship in useAddEventWithRelationship

Copy link

github-actions bot commented Jan 8, 2025

@simonadomnisoru simonadomnisoru marked this pull request as ready for review January 8, 2025 09:10
@simonadomnisoru simonadomnisoru requested a review from a team as a code owner January 8, 2025 09:10
@simonadomnisoru simonadomnisoru changed the title feat: [DHIS2-18018] show related stages widget in view event page feat: [DHIS2-18018][DHIS2-17853] show related stages widget in view event page Jan 27, 2025
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!
I'm getting some strange behavior if the relationship is not bidirectional. It seems to not update/navigate correctly in my app. Could you take a look and see if you get the same conclusion in your env? Otherwise, this looks great!

@@ -26,6 +28,10 @@ export const DefaultPageLayout: PageLayoutConfig = {
type: WidgetTypes.COMPONENT,
name: 'EditEventWorkspace',
},
{
type: WidgetTypes.COMPONENT,
name: 'WidgetRelatedStagesWorkspace',
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 change this name to just RelatedStagesWorkspace? None of the other names contains widget, so just trying to stay consistent. (Sorry, this was probably done ages ago)

@simonadomnisoru simonadomnisoru marked this pull request as draft January 29, 2025 12:26
@simonadomnisoru
Copy link
Contributor Author

Hey @simonadomnisoru! I'm getting some strange behavior if the relationship is not bidirectional. It seems to not update/navigate correctly in my app. Could you take a look and see if you get the same conclusion in your env? Otherwise, this looks great!

Hi @eirikhaugstulen, I've added a success alert which I hope will make the flow less confusing when the relationship is not bi-directional. Let me know if it looks ok now. Thanks for your feedback!

@simonadomnisoru simonadomnisoru marked this pull request as ready for review January 29, 2025 14:43
Copy link
Contributor

@henrikmv henrikmv left a comment

Choose a reason for hiding this comment

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

After deleting a (unidirectional or directional) relationship, the Linked Event widget reappears for the event. Trying to create a new relationship causes some unexpected behaviour:

  1. Trying the Schedule or Enter Details options gives me a 409 error.

  2. The Schedule, Enter Details and Link button is sometimes disabled when all required fields have values (see picture). I'm struggling to provide clear reproduction steps, the behaviour seems inconsistent.

Skjermbilde 2025-02-03 kl  21 31 30

Could you take a look?

@simonadomnisoru simonadomnisoru marked this pull request as draft February 4, 2025 11:39
@simonadomnisoru
Copy link
Contributor Author

After deleting a (unidirectional or directional) relationship, the Linked Event widget reappears for the event. Trying to create a new relationship causes some unexpected behaviour:

  1. Trying the Schedule or Enter Details options gives me a 409 error.
  2. The Schedule, Enter Details and Link button is sometimes disabled when all required fields have values (see picture). I'm struggling to provide clear reproduction steps, the behaviour seems inconsistent.
    Could you take a look?

Hi @henrikmv
I pushed a fix for the 409 error, could you please have a look at it? Regarding the inconsistency in the disabled button behavior, I decided to remove that logic and use the loading logic while processing the API request (just as we do in other forms). I'll have the PR reviewed by someone on the design team as well.
Thank you for the feedback!

@simonadomnisoru simonadomnisoru marked this pull request as ready for review February 6, 2025 11:03
Copy link
Contributor

@henrikmv henrikmv left a comment

Choose a reason for hiding this comment

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

Well done Simona removing the 409 error!

Regarding the inconsistency in the disabled button behavior, removing the disabling function appears to introduce a bug. Since the user can click the button before all required values are set, the loading spinner spins indefinitely due to the missing values.

Skjermbilde 2025-02-06 kl  17 41 24

Would you like to take a look? Thanks!

Copy link

sonarqubecloud bot commented Feb 7, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
5 New issues
5 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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

Successfully merging this pull request may close these issues.

3 participants