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

Categories hierarchy : Fix #411 #418

Merged
merged 12 commits into from
Aug 29, 2023

Conversation

nabilatrhea
Copy link
Contributor

@nabilatrhea nabilatrhea commented Aug 1, 2023

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the COMET-WEB code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

Fix #411
Display categorties hierarchy using z-diagram (needs some adjustments to display the diagram on the right of the datagrid)

image

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Merging #418 (ad424cf) into development (dcdc296) will increase coverage by 0.11%.
The diff coverage is 86.13%.

@@               Coverage Diff               @@
##           development     #418      +/-   ##
===============================================
+ Coverage        78.46%   78.57%   +0.11%     
===============================================
  Files              304      309       +5     
  Lines             6817     6918     +101     
  Branches           697      704       +7     
===============================================
+ Hits              5349     5436      +87     
- Misses            1257     1269      +12     
- Partials           211      213       +2     
Files Changed Coverage Δ
...app/Components/ReferenceData/CategoriesTable.razor 83.33% <0.00%> (-7.58%) ⬇️
.../Components/ReferenceData/ParameterTypeTable.razor 100.00% <ø> (ø)
...mponents/ReferenceData/CategoryNodeComponent.razor 60.00% <60.00%> (ø)
...ts/ReferenceData/CategoryHierarchyDiagram.razor.cs 73.07% <73.07%> (ø)
...mponents/ReferenceData/CategoriesTableViewModel.cs 65.94% <87.50%> (+0.97%) ⬆️
...ReferenceData/CategoryHierarchyDiagramViewModel.cs 98.00% <98.00%> (ø)
...METwebapp/Components/ReferenceData/CategoryNode.cs 100.00% <100.00%> (ø)
...nents/ReferenceData/CategoryNodeComponent.razor.cs 100.00% <100.00%> (ø)

@samatstariongroup
Copy link
Member

samatstariongroup commented Aug 2, 2023

please use semantics from UML (should have been more explicit in the original feature request)

the direction of the arrow is from subclass to superclass and the arrow is an open arrow, see example image below (the "extends"word does not need to be in the arrow)

image

Some more requests: in case the Category IsAbstarct, set the caption to Italics. In case the category is deprecated, use dotted line as style for box

@nabilatrhea
Copy link
Contributor Author

@samatrhea Yes, i have added the Open arrow. however it is not displayed (COMETwebapp/wwwroot/images/UML_Open_arrow.svg) i will try to fix that and add the Italic caption for the abstract categories

Copy link
Member

@samatstariongroup samatstariongroup left a comment

Choose a reason for hiding this comment

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

I have the impression that in the current solution the arrows are pointing in the wrong direction. The direction should be from subclass to superclass: subclass ---> superclass; please have a look at the diagram i provided as an example

@nabilatrhea
Copy link
Contributor Author

@samatrhea
image
image
image

@samatstariongroup
Copy link
Member

@samatrhea image image image

ok,. so the direction is then indeed correct, its is typical to have the most abstract class on top and the subclasses below. The diagram reads a bit contra intuitiv -> it needs to be flipped

@nabilatrhea nabilatrhea force-pushed the feat/GH411-categories-hierarchy branch from 7854ac7 to 5864c94 Compare August 28, 2023 07:39
Copy link
Member

@samatstariongroup samatstariongroup left a comment

Choose a reason for hiding this comment

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

minor editorial change of formatting

@samatstariongroup samatstariongroup requested review from antoineatstariongroup and removed request for lxatstariongroup August 29, 2023 08:21
Copy link
Contributor

@antoineatstariongroup antoineatstariongroup left a comment

Choose a reason for hiding this comment

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

Some small editorial problem and a possible improvement.
Global question: What happen on the diagram if one of the category in the category hierarchy is deprecated?

@nabilatrhea
Copy link
Contributor Author

Some small editorial problem and a possible improvement. Global question: What happen on the diagram if one of the category in the category hierarchy is deprecated?

Now if a category is deprecated, the diagram is updated also on session refresh

@sonarcloud
Copy link

sonarcloud bot commented Aug 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

86.8% 86.8% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@nabilatrhea nabilatrhea merged commit 8547461 into development Aug 29, 2023
9 checks passed
@nabilatrhea nabilatrhea deleted the feat/GH411-categories-hierarchy branch August 29, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Categorization] page - display category hierachies
4 participants