MMS4 Support, Branding Service, Configuration File#141
MMS4 Support, Branding Service, Configuration File#141dlamoris merged 41 commits intoOpen-MBEE:ve_mms4_backport_osfrom
Conversation
Release/3.4.0
Release 3.5.2
|
This pull request introduces 5 alerts and fixes 1 when merging 742072f into 1f70ad2 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 4 alerts and fixes 1 when merging 87aa4a2 into 1f70ad2 - view on LGTM.com new alerts:
fixed alerts:
|
…nto feature/mms4 # Conflicts: # Gruntfile.js # app/mms.html # src/directives/Utils.js # src/directives/mmsSpec.js # src/directives/mmsTranscludeArt.js # src/services/AuthorizationService.js # src/services/ElementService.js # src/services/ProjectService.js # src/services/URLService.js # src/services/ViewService.js
|
I've now merged and incorporated the changes from the BA branch |
|
This pull request introduces 4 alerts and fixes 2 when merging 4fcfe2e into 1f70ad2 - view on LGTM.com new alerts:
fixed alerts:
|
…mms4 # Conflicts: # src/services/AuthorizationService.js
|
This pull request introduces 4 alerts and fixes 1 when merging 148e657 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 4 alerts and fixes 1 when merging 13ed472 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 4 alerts and fixes 1 when merging 50e0f91 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
|
This is now tested and working to the same level as the BA branch. I also backed out most of the OMG specific changes |
|
There is an open issue which will throw errors on user lookup until Open-MBEE/exec-mms#173 is merged and deployed |
|
This pull request introduces 4 alerts and fixes 1 when merging c3c4848 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
dlamoris
left a comment
There was a problem hiding this comment.
first pass, main thing is using the http interceptor instead of passing headers for each call, also with the ckeditor upgrade i don't remember if there's anything else we do, i think the config file was commented out because it depends on where ve actually gets deployed, stuff in there may be incorporated into the directive? guess we'll see
| controller: 'ToolbarCtrl' | ||
| }, | ||
| 'footer@': { | ||
| template: '<ve-footer mms-footer="footer" ng-if="ve_footer"></ve-footer>', |
There was a problem hiding this comment.
i think ve_footer gets set at rootscope? not sure if it's supposed to use env file or other stuff?
There was a problem hiding this comment.
ve-footer is now a separate controller which is why it's set there
| url: '/document/:documentId', | ||
| resolve: { | ||
| documentOb: ['$stateParams', '$q', 'ElementService', 'ViewService', 'refOb', 'ticket', function($stateParams, $q, ElementService, ViewService, refOb, ticket) { | ||
| projectOb: ['$stateParams', 'ProjectService', 'token', function($stateParams, ProjectService, token) { |
There was a problem hiding this comment.
this would have just inherited from parent state, i don't think it's needed here, only needs to be redefined if it's to overwrite parent state's object
| // } | ||
| // return deferred.promise; | ||
| // }; | ||
| var getMetatypes = function(projectId, refId) { |
There was a problem hiding this comment.
none of these will work since there's no more pure elastic query for search
| ticket = t; | ||
| }; | ||
|
|
||
| var getAuthorizationHeaderValue = function() { |
There was a problem hiding this comment.
these were needed by the http interceptor
| * @param {string} site Site name (not title!). | ||
| * @returns {string} The path for site dashboard. | ||
| */ | ||
| var getSiteDashboardURL = function(site) { |
There was a problem hiding this comment.
this is no longer relevant
| deferred.resolve(success.data.username); | ||
| URLService.setToken(token); | ||
| $http.get(URLService.getCheckTokenURL(),URLService.getRequestConfig()).then(function (success) { | ||
| deferred.resolve(success.data); |
There was a problem hiding this comment.
was whatever is calling this handling the changed data accordingly?
|
|
||
| var getLogoutURL = function() { | ||
| return root + '/api/login/ticket/' + ticket; | ||
| var getCheckTokenURL = function(t) { |
There was a problem hiding this comment.
i think there's a bunch of duplicates of this now - i've seen checkLogin, checkToken, checkSession
|
Thanks Doris. I think the interception differential was due to my older changes and me not being aware of it. I’ll make that change and look at the others on Monday |
|
This pull request introduces 4 alerts and fixes 1 when merging 48d44c0 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 6 alerts and fixes 1 when merging 8a7a778 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
src/services/ConfigService.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
I don't think this service is used anywhere
| elements: [postElem], | ||
| source: ApplicationService.getSource() | ||
| }, {timeout: 60000}) | ||
| }, Object.assign({timeout: 60000})) |
There was a problem hiding this comment.
not sure why object.assign is added
There was a problem hiding this comment.
Leftover from when I added headers at that point rather than in the interceptor
app/config/config.example.js
Outdated
| @@ -0,0 +1,40 @@ | |||
| (function (window) { | |||
There was a problem hiding this comment.
i tried deploying it, i think the window arg and 'this' at the end are messing it up (undefined), should be fine if they're just taken out
There was a problem hiding this comment.
Will not work for me without those statements. Lets setup some time to talk about it
There was a problem hiding this comment.
I have committed the necessary changes
|
this version is on https://opencae-uat.jpl.nasa.gov/mms4ve/mms.html currently, talking to cae-mms-uat-lb.jpl.nasa.gov |
|
This pull request introduces 6 alerts and fixes 1 when merging 09016a2 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 6 alerts and fixes 1 when merging 5ebb561 into 9cccfc9 - view on LGTM.com new alerts:
fixed alerts:
|
This update adds