-
Notifications
You must be signed in to change notification settings - Fork 15
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
GDB-6329 - add new license is expired message #618
Conversation
…QL queries to work
…QL queries to work
…t-AD/graphdb-workbench into GDB-6329-add_expired_license_msg � Conflicts: � src/js/angular/explore/controllers.js � src/js/angular/jdbc/controllers.js � src/js/angular/namespaces/controllers.js � src/pages/jdbc-create.html
…, run skipped test
…QL queries to work
…, run skipped test
…w message visible. Code formatting corrections.
…' into GDB-6329-add_expired_license_msg # Conflicts: # src/js/angular/explore/controllers.js # src/js/angular/graphexplore/directives/rdf-class-hierarchy.directive.js # src/js/angular/jdbc/controllers.js # src/js/angular/namespaces/controllers.js # test/namespaces/controllers.spec.js # test/resource/controllers.spec.js
src/js/angular/graphexplore/controllers/dependencies-chord.controller.js
Outdated
Show resolved
Hide resolved
…to backend. Corrected strings for translation service.
Jenkins retry build |
… rdf-class-hierarchy.directive.js, because latter has concurrency issues with the
…' into GDB-6329-add_expired_license_msg
# Conflicts: # package-lock.json
SonarCloud Quality Gate failed. |
@@ -23,6 +23,8 @@ function licenseService($rootScope, LicenseRestService, $translate) { | |||
that.loadingLicense = false; | |||
updateProductType(that.license); | |||
}); | |||
}). finally(function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the extra space before finally
|
||
window.addEventListener('load', function () { | ||
that.repository = { | ||
id: localStorage.getItem(that.repositoryStorageName) || '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the adapter from src/js/angular/utils/local-storage-adapter.js! You can also move the keys in there as constants.
const initView = function () { | ||
if (!$scope.getActiveRepository()) { | ||
if (!$scope.getActiveRepository() || | ||
!$licenseService.isLicenseValid()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the method above
@@ -339,6 +339,10 @@ function searchResourceInput($location, toastr, ClassInstanceDetailsService, Aut | |||
} | |||
}); | |||
|
|||
$scope.isLicenseValid = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this method. As far as I can see the license status is loaded once and is stored in the license service, so in each $licenseService.isLicenseValid() call you get the same status. That being said, this additional method in each controller where you need to check the status is just meaningless and also using it in directive templates is quite overwhelming than using a plain variable binding.
So, can you consider resolving the license status in a boolean flag where you need it and use it instead of this method. @sava-savov-ontotext what do you think?
@@ -10,13 +10,18 @@ const CLASS_HIERARCHY = 'class hierarchy'; | |||
describe('Class hierarchy screen validation', () => { | |||
let repositoryId; | |||
|
|||
beforeEach(() => { | |||
before(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why you changed beforeEach
and afterEach
hooks here and in the rest of the tests?
ClassViewsSteps.verifyGraphIsDisplayed(NEWS_GRAPH); | ||
verifyCounterValue(CLASS_COUNT_OF_NEWS_GRAPH); | ||
ClassViewsSteps.clickGraphBtn() | ||
.then(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you chain here?
// Close the menu to avoid overlapping other elements | ||
cy.get('.dropdown-toggle').click(); | ||
.click() | ||
.then(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly would suggest you to avoid this chaining approach.
@sava-savov-ontotext will take over the PR and move it forward. |
@sava-savov-ontotext Is this still valid? Can we close this MR? |
Closing this MR due to outdated changes. New one opened: #1380 |
What?
Added a new "license is expired message" to the views that need SPARQL queries to work. Removed toastr message from views where it was not needed.
Why?
A few of the views were not showing the proper message in the case of expired GDB license. There were others, which showed both the new message and the old one, so I hid the old one where necessary.
How?
I used the new
licenseService
for validations in html and js files.