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

[58427] Indicate that there are work package relations which the user has no permission to see #17604

Conversation

bsatarnejad
Copy link
Contributor

@bsatarnejad bsatarnejad commented Jan 14, 2025

Ticket

https://community.openproject.org/wp/58427
https://community.openproject.org/wp/60957
https://community.openproject.org/wp/60958
https://community.openproject.org/wp/61044

What are you trying to accomplish?

We want to show ghost relations of a WP in relations tab, but they will look different from visible relations.

Screenshots

Screenshot 2025-01-15 at 14 01 13

What approach did you choose and why?

Return both visible and ghost relations to index component and then call relation component with different values for visible and ghost relations.

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

cbliard and others added 5 commits January 13, 2025 16:04
The mediator is responsible for providing the relations and children
from a work package for the relations tab. This moves the responsibility
of relations and children visibility in one central place.
…rning visible, invisible relations and their type
@bsatarnejad bsatarnejad self-assigned this Jan 14, 2025
@cbliard cbliard force-pushed the code-maintenance/introduce-relations-tab-mediator branch from a10a0be to b0ee80c Compare January 15, 2025 15:03
Base automatically changed from code-maintenance/introduce-relations-tab-mediator to dev January 15, 2025 15:25
Copy link
Member

@cbliard cbliard left a comment

Choose a reason for hiding this comment

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

Nice work 👏

I only have two tiny code change requests.

I have also some 2 other questions / remarks about things that are not specified:

  • For milestone work packages, I think it should show only one date like on the full page view
  • For predecessor / successor ghost relation, when the dates are not set, I think the date line should still be shown, similar to what happens for ghost children without dates.

I asked both questions to Parimal in the related work package (#58427).

config/initializers/menus.rb Show resolved Hide resolved
bsatarnejad and others added 4 commits January 29, 2025 13:03
…b to use visible_relations instead of @visible_relations

Co-authored-by: Christophe Bliard <[email protected]>
…b to use visible_children instead of @ visible_children

Co-authored-by: Christophe Bliard <[email protected]>
…lations-which-the-user-has-no-permission-to-see
@bsatarnejad bsatarnejad requested a review from cbliard January 29, 2025 15:34
Copy link
Member

@cbliard cbliard left a comment

Choose a reason for hiding this comment

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

Thanks, I found some issues:

  1. In the bottom of the page, there is the number of relations being displayed. Not sure why it happens, but it's a bug. I simply load the page with the relations, and this gimmick appears.

image

  1. For the dates widget, when there are no dates, a dash should be displayed. Currently nothing is displayed

image

It happens because of the condition start_date == due_date to display a single date. The condition is also true when both are nil.

Relevant text from the spec in 58427:

if a date missing (start date and/or finish date) we will nevertheless show the relevant icon with a dash (-) to indicate what is missing.

So I'll mark "review failed" for Feature: Indicate that there are work package relations which the user has no permission to see (#58427) for these two reasons.

  1. The icon for manually/automatically scheduled is displayed correctly for predecessors / successors, but not for children where the calendar icon is always displayed. Both should behave the same. I think the code to handle is currently duplicated for successors/predecessors and children and could be extracted into a method or a component so that it always behaves the same.

So I'll mark "review failed" for Bug: Icon beside dates of a relation is wrong in relation row (#60958)

And I'll mark "review approved" for both Feature: Relations tab: Show a single date when start and finish dates are the same (#61044) and Bug: Show dates of Children of a work package in relations tab (#60957).

@bsatarnejad
Copy link
Contributor Author

Thanks, I found some issues:

  1. In the bottom of the page, there is the number of relations being displayed. Not sure why it happens, but it's a bug. I simply load the page with the relations, and this gimmick appears.

image

  1. For the dates widget, when there are no dates, a dash should be displayed. Currently nothing is displayed

image

It happens because of the condition start_date == due_date to display a single date. The condition is also true when both are nil.

Relevant text from the spec in 58427:

if a date missing (start date and/or finish date) we will nevertheless show the relevant icon with a dash (-) to indicate what is missing.

So I'll mark "review failed" for Feature: Indicate that there are work package relations which the user has no permission to see (#58427) for these two reasons.

  1. The icon for manually/automatically scheduled is displayed correctly for predecessors / successors, but not for children where the calendar icon is always displayed. Both should behave the same. I think the code to handle is currently duplicated for successors/predecessors and children and could be extracted into a method or a component so that it always behaves the same.

So I'll mark "review failed" for Bug: Icon beside dates of a relation is wrong in relation row (#60958)

And I'll mark "review approved" for both Feature: Relations tab: Show a single date when start and finish dates are the same (#61044) and Bug: Show dates of Children of a work package in relations tab (#60957).

Thanks @cbliard , I'll fix them 👍

@bsatarnejad bsatarnejad requested a review from cbliard January 30, 2025 12:27
Copy link
Member

@cbliard cbliard left a comment

Choose a reason for hiding this comment

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

Perfect 👌

@cbliard cbliard merged commit 0034c17 into dev Jan 30, 2025
14 checks passed
@cbliard cbliard deleted the 58427-indicate-that-there-are-work-package-relations-which-the-user-has-no-permission-to-see branch January 30, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants