This repository has been archived by the owner on Jan 7, 2020. It is now read-only.
New Feature - Joint Public Comment Period #119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Includes: * EPIC-960 Joint PCP - Can Download Comments * EPIC-961 Joint PCP - CEAA: Can download Comments * EPIC-979 Joint PCP - Mandatory name and location * EPIC-931 Joint PCP - Feature Implementation: Submit a Comment * EPIC-932 Joint PCP - Modify PCP Creation Page to Joint OR Regular * EPIC-935 Joint PCP - Hook up Uploader * EPIC-937 Joint PCP - Public View * EPIC-976 PCP (and Joint PCP) - Banner Modifications * EPIC-1015 Joint PCP - comment details dialog does not show attachments for public users * EPIC-1017 Joint PCP - Attachment icon not showing on PCP comment display page * EPIC-1021 Joint PCP - CEAA users cannot see Rejected comments on PCP comment display page * EPIC-1024 Joint PCP CEAA role CSV download. Need to list rejected documents. * EPIC-1026 Joint PCP - comment details dialog for CEEA users
marklise
reviewed
May 30, 2017
| // | ||
| // ------------------------------------------------------------------------- | ||
| comment2 : { type:String, default:'' }, |
There was a problem hiding this comment.
These not not named very well - be more descriptive.
| @@ -110,6 +110,10 @@ module.exports = function (app) { | |||
| .get (routes.setAndRun (CommentModel, function (model, req) { | |||
| return model.getCommentDocuments(req.params.commentId); | |||
| })); | |||
| app.route ('/api/comment/:commentId/documents2').all(policy ('guest')) | |||
There was a problem hiding this comment.
API endpoint is not named very well - be more descriptive.
| period.phaseName = project.currentPhase.name; | ||
|
|
||
| CommentPeriodModel.add($scope.period) | ||
| .then(function (model) { |
There was a problem hiding this comment.
Indentation of the .then should line up with leading edge of 'CommentPeriodModel'.
| $scope.busy = true; | ||
|
|
||
| CommentPeriodModel.save($scope.period) | ||
| .then(function (model) { |
|
|
||
| // Create functions bound to the specific collection we will be adding files to... | ||
| // These conform to the signature of the callback expected by the "Link Files" modal dialog | ||
| $scope.addFilesToPackage1 = function (data) { addFilesToCollection(period.relatedDocuments, data); }; |
There was a problem hiding this comment.
For joint, package 1=EAO and package 2=CEAA. Naming should be clear, avoid use of numbers. Better to use a generic function and pass in the package to add data to (deal with where to put things based on input param.
| s.changeFilterCommentPackage = function() { | ||
| s.isLoading = true; | ||
| s.filterCommentPackage = s.selectedCommentFilterOption.name; | ||
| console.log("Change package filter", s.filterCommentPackage); |
There was a problem hiding this comment.
Remove all console.logs unless debug flag is set (or just remove it entirely).
| } | ||
| }, | ||
| controller: function ($scope, $modalInstance, docs) { | ||
| controller: function ($scope, $modalInstance, docs, docs2) { |
There was a problem hiding this comment.
Variable naming should be more descriptive.
| if (docCount === 0 ) { | ||
| // We don't need to do anything but add the comment. | ||
| // console.log("s.comment:", s.comment); | ||
| CommentModel.add (s.comment) |
| }, function (response) { | ||
| if (response.status > 0) { | ||
| // docUpload.errorMsg = response.status + ': ' + response.data; | ||
| console.log("error data:",response.data); |
| }) | ||
| .then (function (comment) { | ||
| ctrl.isBusy = false; | ||
| console.log("go to next stage"); |
* Fix whitespace and indentation * Remove commented code * Remove all console.logs * Improve variable naming
* Improve variable naming for Joint PCP documents and comments * Rename API endpoint
|
@marklise - I have addressed all your comments. Please review. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes: