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

Fix problems displaying external links #944

Merged
merged 2 commits into from
Mar 4, 2020
Merged

Conversation

osma
Copy link
Member

@osma osma commented Mar 3, 2020

by more judicious use of isExternal and getExVocab. Fixes #935

ConceptMappingPropertyValue.isExternal() is removed because it is no longer being used.

@osma osma added the bug label Mar 3, 2020
@osma osma added this to the 2.4 milestone Mar 3, 2020
@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #944 into master will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #944      +/-   ##
============================================
+ Coverage     58.27%   58.38%   +0.10%     
+ Complexity     1490     1486       -4     
============================================
  Files            32       32              
  Lines          4168     4162       -6     
============================================
+ Hits           2429     2430       +1     
+ Misses         1739     1732       -7     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c769247...819bfaa. Read the comment docs.

@osma osma requested a review from kouralex March 3, 2020 14:20
Copy link
Contributor

@kouralex kouralex left a comment

Choose a reason for hiding this comment

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

There is still this line

{% if propval.isExternal %}
in twig template that still uses the old code that must be taken care of. Other than that, looks good!

@osma
Copy link
Member Author

osma commented Mar 4, 2020

in twig template that still uses the old code that must be taken care of

This is as it should be. ConceptPropertyValue still has an isExternal method. It was only removed from ConceptMappingPropertyValue where it was not used anymore.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 4, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell B 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@osma
Copy link
Member Author

osma commented Mar 4, 2020

Scrutinizer reports bogus code duplication issues - not really a problem with the code.

SonarCloud reports code smells because of repeated literal strings in tests - not a problem and we have many of these already in the test suite.

@osma osma requested a review from kouralex March 4, 2020 11:02
Copy link
Contributor

@kouralex kouralex left a comment

Choose a reason for hiding this comment

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

Now this looks good to me. The original case is solved and unnecessary code has been removed, great work!

@osma osma merged commit bd37701 into master Mar 4, 2020
@osma osma deleted the issue935-invalid-external-links branch March 4, 2020 11:26
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.

Invalid links to external URIs in non-mapping properties
2 participants