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

[1077] Fix explorer in case of non-sysml domain #1071

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dvojtise
Copy link
Contributor

@dvojtise dvojtise commented Mar 1, 2025

Fix behavior of hasChildren management of syson explorer when handling model elements from non-sysml domains.
It now falls back to SiriusWeb implementation when necessary.

PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW

Project management

  • Has the pull request been added to the relevant milestone?
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, type:)
  • Have the relevant issues been added to the same project milestone as the pull request?

Changelog and release notes

  • Has the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
  • In case of a change with a visual impact, are there any screenshots in the doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
  • In case of a key change, has the change been added to Key highlights section in doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?

Documentation

  • Have you included an update of the documentation in your pull request? Please ask yourself if an update (installation manual, user manual, developer manual...) is needed and add one accordingly.

Tests

  • Is the code properly tested? Any pull request (fix, enhancement or new feature) should come with a test (or several). It could be unit tests, integration tests or cypress tests depending on the context. Only doc and releng pull request do not need for tests.

@dvojtise dvojtise force-pushed the fix-explorer-on-non-sysml-domains branch 2 times, most recently from ee6db2e to 0b4050f Compare March 1, 2025 22:56
@AxelRICHARD AxelRICHARD added this to the 2025.4.0 milestone Mar 2, 2025
@AxelRICHARD
Copy link
Member

Could you please create a bug describing the issue?
Then:

  • replace [bug] by [ISSUE_NUMBER] in the commit message title.
  • update the changelog (just look latest commits to see how it is done)
  • update the release note (just look latest commits to see how it is done)

Regards,

@dvojtise dvojtise changed the title [bug] Fix explorer in case of non-sysml domain [1077] Fix explorer in case of non-sysml domain Mar 4, 2025
@dvojtise
Copy link
Contributor Author

dvojtise commented Mar 4, 2025

done 🙂

Regards,

@dvojtise dvojtise force-pushed the fix-explorer-on-non-sysml-domains branch 2 times, most recently from ca14ddf to 5210b43 Compare March 4, 2025 21:56
Copy link
Member

@AxelRICHARD AxelRICHARD left a comment

Choose a reason for hiding this comment

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

Could you please add an integration test or a cypress test?
An also add
Bug: https://github.com/eclipse-syson/syson/issues/1077
just above the signed-off-by line? (see other commits for examples)

Thank you!

public SysONDefaultExplorerServices(IIdentityService identityService, IContentService contentService, IRepresentationMetadataSearchService representationMetadataSearchService, IExplorerServices explorerServices,
ISysONExplorerFilterService filterService) {
ISysONExplorerFilterService filterService, ExplorerServices siriuswebExplorerService) {
Copy link
Member

Choose a reason for hiding this comment

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

You don't need ExplorerServices from Sirius Web.
There is already a IExplorerServices explorerServices variable that gives you access to hasChildren method.
We don't have any implementation of IExplorerServices in SysON so the only implementation returned by IExplorerServices explorerServices is the one coming from Sirius Web.

CHANGELOG.adoc Outdated
@@ -63,6 +65,7 @@ Export to textual SysMLv2 is not fully implemented yet so there are still unhand
- https://github.com/eclipse-syson/syson/issues/1020[#1020] [general-view] The multiplicity should not be displayed on edges.
- https://github.com/eclipse-syson/syson/issues/1009[#1009] [metamodel] Fix an issue where the diagram direct edit on graphical nodes could raise a backend error on unsettable enum attributes.
- https://github.com/eclipse-syson/syson/issues/1052[#1052] [general-view] Fix an issue where the execution of "New Port In/Inout/Out" tools was failing.
- https://github.com/eclipse-syson/syson/issues/1077[#1077] [explorer] Fix navigation in case of project containing both sysml and non-sysml model elements
Copy link
Member

Choose a reason for hiding this comment

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

You've added the entry twice in the changelog

@dvojtise dvojtise force-pushed the fix-explorer-on-non-sysml-domains branch 3 times, most recently from 2039a04 to 2b1bbab Compare March 5, 2025 21:41
@dvojtise
Copy link
Contributor Author

dvojtise commented Mar 5, 2025

I've applied your suggestions.
except for the test: I'm a bit lost about how to properly write integration test or cypress test:

the default Syson application doesn't propose any additional metamodel (not even Domain) so I don't know how to use the default UI to do that.
The proposed fix works for metamodel + edit added as spring jar and I don't see which metamodel would make sense to add to syson as default for every syson users.

note: creating model conformant to domain added using sirius web studio doesn't work, but for another reason I haven't investigated : the create object of the explorer show the elements it can create but does nothing without raising any error

Fix behavior of hasChildren management of syson explorer when handling model elements from non-sysml domains. It now falls back to SiriusWeb implementation when necessary.

Bug: eclipse-syson#1077
Signed-off-by: Didier Vojtisek <[email protected]>
@dvojtise dvojtise force-pushed the fix-explorer-on-non-sysml-domains branch from 2b1bbab to c71da9e Compare March 6, 2025 14:38
@dvojtise
Copy link
Contributor Author

dvojtise commented Mar 6, 2025

I added a spring test

note: it uses mockito (that is already imported by spring-boot-starter-test) to create mock for some of the services
however, mockito seems to require a maven-surefire-plugin configuration to get rid of a warning (about self-attaching to enable the inline-mock-maker)

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.

Explorer not correctly expanding content of project with both sysml and non-sysml content
2 participants