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.

Sign up for GitHub

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

Jump to bottom

178 link from system modeller UI to domain model docs #211

Merged
merged 29 commits into from
Jan 10, 2025
Merged

178 link from system modeller UI to domain model docs #211

merged 29 commits into from
Jan 10, 2025

Conversation

kenmeacham
Copy link
Contributor

No description provided.

@kenmeacham kenmeacham self-assigned this Oct 25, 2024
@kenmeacham kenmeacham linked an issue Oct 25, 2024 that may be closed by this pull request
Link from system-modeller UI to domain model docs #178
Closed
@kenmeacham
Copy link
Contributor Author

I had a thought, as I was testing. If the user is browsing a system model that has been imported, and uses a different system model, I presume the doc link should be wrt that version, rather than simply the installed domain model version.

@mike1813 - do you agree?

@kenmeacham
Copy link
Contributor Author

I had a thought, as I was testing. If the user is browsing a system model that has been imported, and uses a different system model, I presume the doc link should be wrt that version, rather than simply the installed domain model version.

@mike1813 - do you agree?

After today's discussion, @mike1813 generally agreed. This at least ensures that the GUI links through the to the correct domain concept (doc page), wrt the "validated" system model. However, we still have an invalid system model (i.e. validated with a different version than what is currently installed, so @mike1813 suggested perhaps an additional warning to the user, prior to being redirected to the doc page.

@kenmeacham
#178: Fix security issue
bbbc867
@kenmeacham
#178: Consolidate code to reduce duplication
cb37de6
@kenmeacham
#178: Make duplicated regex string a constant
44a499b
@kenmeacham
#178: Replace hidden tabs in queries with spaces (java:S2479)
e9bcd9d
@kenmeacham
#178: Remove unused imports
7c22acc
@kenmeacham
#178: Use @GetMapping (TODO - refactor other methods)
32e4b2c
@kenmeacham
#178: Remove "throws UnexpectedException". Seems unnecessary
9fea8c4
@kenmeacham
#178: Add Javadocs to new methods
eb2b69f
@kenmeacham
Merge branch 'dev' into 178-link-from-system-modeller-ui-to-domain-mo…
864bbc8 
…del-docs

# Conflicts:
#	src/main/java/uk/ac/soton/itinnovation/security/systemmodeller/semantics/ModelObjectsHelper.java
@kenmeacham
#178: Fixed KNOWLEDGEBASE_DOCS_QUERY_URL in docker-compose.test.yml. …
6b5f0cc 
…Also removed redundant version
@kenmeacham
#178: Modified to use validatedDomainModelVersion instead of domainMo…
86b4125 
…delVersion
@kenmeacham
#178: Implement ConfirmDocRedirectModal to warn user about mismatch i…
10b2f21 
…n domain model versions
@kenmeacham
#178: Add propTypes
f4ff05b
@kenmeacham
Copy link
Contributor Author

I had a thought, as I was testing. If the user is browsing a system model that has been imported, and uses a different system model, I presume the doc link should be wrt that version, rather than simply the installed domain model version.
@mike1813 - do you agree?

After today's discussion, @mike1813 generally agreed. This at least ensures that the GUI links through the to the correct domain concept (doc page), wrt the "validated" system model. However, we still have an invalid system model (i.e. validated with a different version than what is currently installed, so @mike1813 suggested perhaps an additional warning to the user, prior to being redirected to the doc page.

@mike1813 - I've implemented a pop-up warning, as discussed (see below). Do you think the wording is clear?

image

@kenmeacham
Copy link
Contributor Author

We agreed that @panositi would do some functional testing of this branch, and that @danshearer would have a look through the code changes. Any questions, let me know..

@kenmeacham
Copy link
Contributor Author

@mike1813 suggested a small improvement to the wording in the pop-up. Now looks like this:

image

panositi
panositi approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@panositi panositi left a comment

Choose a reason for hiding this comment

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

Functionality tested and appears to work correctly.

@kenmeacham kenmeacham requested a review from barthasanyi January 9, 2025 10:46
@danshearer
Copy link
Member

We agreed that @panositi would do some functional testing of this branch, and that @danshearer would have a look through the code changes. Any questions, let me know..

@kenmeacham sorry for being slow.

The code changes look ok to me.

A detail: Sonar is complaining about commented-out log lines. That's usually better done by using a log level instead.

@danshearer danshearer closed this Jan 10, 2025
@danshearer
Copy link
Member

@kenmeacham I accidentally closed this!

@danshearer danshearer reopened this Jan 10, 2025
barthasanyi
barthasanyi approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@barthasanyi barthasanyi left a comment

Choose a reason for hiding this comment

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

The code looks fine to me -- but I am not familiar with every part of the Spyderisk codebase.

danshearer
danshearer approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@danshearer danshearer left a comment

Choose a reason for hiding this comment

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

Looks good as per group chat this morning

@kenmeacham
Merge branch 'dev' into 178-link-from-system-modeller-ui-to-domain-mo…
d06eb41 
…del-docs

# Conflicts:
#	src/main/java/uk/ac/soton/itinnovation/security/systemmodeller/semantics/ModelObjectsHelper.java
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@kenmeacham kenmeacham merged commit 771eb75 into dev Jan 10, 2025
4 checks passed
@kenmeacham kenmeacham deleted the 178-link-from-system-modeller-ui-to-domain-model-docs branch January 10, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danshearer danshearer danshearer approved these changes

@barthasanyi barthasanyi barthasanyi approved these changes

@panositi panositi panositi approved these changes

Assignees

@kenmeacham kenmeacham

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Link from system-modeller UI to domain model docs
4 participants
@kenmeacham @danshearer @barthasanyi @panositi