feature/deseng751: Added external link model, service, and swagger definitions. Updated search service. #83
Conversation
jareth-whitney
commented
Jan 10, 2025
jareth-whitney
commented
- Added external link model, service, and swagger definitions.
- Updated search service.
…finitions. Updated search service.
There was a problem hiding this comment.
Thanks for this! Please see my comments. I'm basically just asking for commented-out code to be removed. Also if you copied code from other controllers to bootstrap you, please just be sure to remove any references to the other models (Comments, CommentPeriods, etc.) if necessary.
Since you may be starting a different ticket, I'll leave this one approved so you can merge it first
api/helpers/models/externalLink.js
Outdated
| projectPhase: { type: String, default: '' }, | ||
| checkbox: { type: Boolean, default: false }, | ||
|
|
||
| // alt: { type: String, default: '' }, |
There was a problem hiding this comment.
Make sure to remove these commented-out lines!
There was a problem hiding this comment.
Apologies, this was commented out in the planning stage while determining which properties were required.
api/controllers/externalLink.js
Outdated
| defaultLog.info("Saved new external link object:", link._id); | ||
| const Comment = mongoose.model('Comment'); | ||
| const c = await Comment.update({ _id: _comment }, { $addToSet: { documents: link._id } }); | ||
| defaultLog.info('External link updated:', c); |
There was a problem hiding this comment.
I'm trying to understand this comment here. Was the external link updated at all? It seems more like the Comment was updated. Maybe some variable names/comments need to be changed?
There was a problem hiding this comment.
Right so this controller is based on the document controller, but modified as needed. In this case, the unProtectedPost is not even being used at the moment. There is a feature in LUP that allows end users to upload files. If we decided to let them upload links, this is where it would be handled. However, because it's not being used, I'm going to remove it from the code.
There was a problem hiding this comment.
Yeah, agreed - it doesn't seem like external links would need that!
api/controllers/externalLink.js
Outdated
| true) // count | ||
| .then((data) => { | ||
| Utils.recordAction('Head', 'ExternalLink', args.swagger.params.auth_payload.preferred_username, args.swagger.params.exLinkId?.value || null); | ||
| // /api/commentperiod/ route, return 200 OK with 0 items if necessary |
There was a problem hiding this comment.
Are you actually hitting the /api/commentperiod route here?
There was a problem hiding this comment.
Comment removed
api/controllers/externalLink.js
Outdated
| defaultLog.info('EXTERNAL LINK PROTECTED POST'); | ||
| try { | ||
| const project = args.swagger.params.project?.value; | ||
| // const _comment = args.swagger.params._comment.value; |
There was a problem hiding this comment.
Is this commented-out code needed?
There was a problem hiding this comment.
Removed... see above comment about figuring out needed properties.
api/controllers/externalLink.js
Outdated
| extLink.description = args.swagger.params.description.value; | ||
| extLink.projectPhase = args.swagger.params.projectPhase.value; | ||
| extLink.checkbox = 'true' === args.body.checkbox ? true : false; | ||
| // extLink._comment = _comment; |
There was a problem hiding this comment.
Is this commented-out code needed?
| obj.dateAdded = args.swagger.params.dateAdded.value; | ||
| obj.dateUpdated = args.swagger.params.dateUpdated.value; | ||
| obj.description = args.swagger.params.description.value; | ||
| obj.section = "null" === args.swagger.params.section.value ? null : args.swagger.params.section.value; |
There was a problem hiding this comment.
I think this would do the same thing. There may be an even shorter way to do it
| obj.section = "null" === args.swagger.params.section.value ? null : args.swagger.params.section.value; | |
| obj.section = args.swagger.params.section.value ? args.swagger.params.section.value : null; |
There was a problem hiding this comment.
Unfortunately I don't think it would work in this situation, since we're looking for a string with the text "null".
There was a problem hiding this comment.
You're right! I remember that sections can be "null" sometimes.
There was a problem hiding this comment.
Yeah I believe it's because everything has to be stringified for the API request and then converted back
… from externalLink controller.
