Conversation
| /** | ||
| * @param dbKey - key with project and company name | ||
| * @param name - name of suite | ||
| * @param version Suite object found by given criteria or null. |
There was a problem hiding this comment.
Please fix this javadoc - move Suite object found by given criteria or null. to @return and add description for version param
| }).toList(); | ||
| } | ||
|
|
||
| public List<String> listSuiteVersions(DBKey dbKey, String name) throws StorageException{ |
There was a problem hiding this comment.
This method should return list of pairs: { version: "..", correlationId: "..} - we can't assume that oldest suite will have version equal to 1 because Cleaner might have deleted it
| return 'http://aet-vagrant' + '/api' + '/history' + '?' + | ||
| $allParametersList[0] + '&' + | ||
| $allParametersList[1] + '&' + | ||
| $allParametersList[2]; |
There was a problem hiding this comment.
The path is wrong when correlationId is used.
Example: /report.html?company=xxx&project=xxx&correlationId=xxx
| .find(Filters.eq(SUITE_PARAM_NAME, name)) | ||
| .sort(Sorts.descending(SUITE_VERSION_PARAM_NAME)); | ||
|
|
||
| return FluentIterable.from(found).transform(new Function<Document, String>() { |
There was a problem hiding this comment.
You may use Java 8 features instead, e.g.:
StreamSupport.stream(found.spliterator(), false)
.map(result -> result.getString("correlationId"))
.collect(Collectors.toList());
1d95203 to
26610db
Compare
4d23c65 to
71aaf63
Compare
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public List<List<String>> listSuiteVersions(DBKey dbKey, String name) throws StorageException { |
There was a problem hiding this comment.
This method should return List<Map<String, String>> or List<SuiteVersion> (where SuiteVersion is a new class with 2 fields - version and correlationId. Then in HistoryServlet you just need to call gson.toJson instead of the whole JsonArray/JsonObject building
tkaik
left a comment
There was a problem hiding this comment.
Please update Report app documentation with new Suite features
malaskowski
left a comment
There was a problem hiding this comment.
Please update also report documentation with example of using history.
| ***************************************/ | ||
|
|
||
| function updateToolbar() { | ||
| document.getElementsByClassName('suite-history-container')[0].style.display = 'none'; |
There was a problem hiding this comment.
You can use jQuery here.
| } | ||
|
|
||
| function buildApiPath($allParametersList) { | ||
| return 'http://aet-vagrant' + '/api' + '/history' + '?' + |
There was a problem hiding this comment.
Please use AET endpoint address defined in the endpointConfigurationService.
| $allParametersList[2]; | ||
| } | ||
|
|
||
| function getSuiteHistory(suiteHeaders, $rootScope, $http) { |
There was a problem hiding this comment.
This kind of logic should be placed into angular service not in the controller.
E.g. look at metadataEndpointService.
| <div class="toolbar-block fontawesome-link" data-toggle="modal" data-target="#helpModal"> | ||
| <i class="fas fa-question-circle fa-lg"></i> | ||
| </div> | ||
| <div class="suite-history-container"> |
There was a problem hiding this comment.
Maybe it would be good idea to put that into separate modal? Like e.g. noteModal (separate controller and view).
| suite = metadataDAO.getSuite(dbKey, correlationId); | ||
| } else if (isValidName(suiteName)) { | ||
| suite = metadataDAO.getLatestRun(dbKey, suiteName); | ||
| if(isValidName(suiteVersion)){ |
There was a problem hiding this comment.
Shouldn't there be method isValidVersion here?
6a20f26 to
f7c1d59
Compare
| * **URL**: `/api/history` | ||
| * **HTTP Method**: GET | ||
| * **Parameters**: `company`, `project`, `suite` | ||
| * **Example**: http://aet.example.com/api/history??company=cognifide&project=example&suite=mysimplesuite |
|
|
||
|  | ||
|
|
||
| Clicking the suite's name in the top toolbar will show a popup in which the user can see every suite's version that is still available in the database. User can also directly go to the newest version of the suite by clicking |
There was a problem hiding this comment.
It looks like unfinished sentence :)
There was a problem hiding this comment.
Right, I don't know what happened there. Fixing now.
| */ | ||
| function historyService($rootScope, $http, endpointConfiguration) { | ||
| var suiteHeaders; | ||
| $rootScope.endpointUrl = endpointConfiguration.getEndpoint().getUrl; |
There was a problem hiding this comment.
Is that necessary to bind this value to the $rootScope variable?
Maybe you can use local variable instead?
There was a problem hiding this comment.
This value isn't used anywhere in the service file - it is used only in the view file so it has to be put in the $rootScope, otherwise it wouldn't be possible to use it in the historyModal.view.html file.
There was a problem hiding this comment.
Where exactly in historyModal.view.html is used endpointUrl?
| suite = metadataDAO.getLatestRun(dbKey, suiteName); | ||
| if(isValidVersion(suiteVersion)){ | ||
| suite = metadataDAO.getSuite(dbKey, suiteName, suiteVersion); | ||
| }else { |
There was a problem hiding this comment.
Please fix formatting here
| var suite = cUrl.searchParams.get('suite'); | ||
| if (suite === null) { | ||
| var correlationId = cUrl.searchParams.get('correlationId'); | ||
| suite = correlationId.split('-')[2]; |
There was a problem hiding this comment.
It is possible that company or project or suite values contain - character - then this line will not work correctly.
I think it can be done like this (probably can be done simpler :))
var cp = company + '-' + project + '-';
var suiteNameWithTimestamp = correlationId.substr(a.indexOf(cp) + cp.length)
// e.g. now: suiteNameWithTimestamp == 'suite-name-123123123` - we need to cut timestamp from here
| var company = suiteHeaders[i].correlationId.split('-')[0]; | ||
| var project = suiteHeaders[i].correlationId.split('-')[1]; | ||
| var suite = suiteHeaders[i].correlationId.split('-')[2]; | ||
| var correlationId = suiteHeaders[i].correlationId.split('-')[3]; |
There was a problem hiding this comment.
I think that you're parsing timestamp from the correlationID here, so this var should be renamed to e.g. timestamp
And as above - we should handle cases where company/project/suite have '-' characters
| var company = company; | ||
| var project = project; | ||
| var suite = suite; | ||
| var timestamp = suiteHeaders[i].correlationId.split('-')[suiteHeaders[i].correlationId.split('-').length - 1]; |
There was a problem hiding this comment.
This is a bit hard to read now, can we make suiteHeaders[i].correlationId.split('-') a variable? e.g.:
var correlationIdParts = suiteHeaders[i].correlationId.split('-');
var timestamp = correlationIdParts[correlationIdParts.length - 1]
| project: project, | ||
| suite: suite, | ||
| version: version, | ||
| correlationId: timestamp, |
There was a problem hiding this comment.
correlationId: timestamp is a bit confusing, please change to timestamp: timestamp (because it's not correlatoinId here)
User can now view and go to any version of that suite.
Description
User is now able to go trough all versions of the suite by clicking on the timestamp.
Suite's versions are displayed after clicking the current version on the top toolbar.
Types of changes
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.