[GSoC19] Articles improvements #7
Conversation
|
Can we remove the files |
|
Can you please add |
app/articles/server/settings.js
Outdated
| import { settings } from '../../settings'; | ||
|
|
||
| const defaults = { | ||
| enable: false, |
There was a problem hiding this comment.
Why is it kept inside a dict, and the dict a separate variable as we just this value at one place and directly? Can we directly use this value instead?
app/articles/server/settings.js
Outdated
| public: true, | ||
| }); | ||
|
|
||
| this.add('Article_Site_title', 'Rocket.Chat', { |
There was a problem hiding this comment.
Article_Site_title > Article_Site_Title
app/articles/server/settings.js
Outdated
|
|
||
| Meteor.startup(() => { | ||
| settings.addGroup('Articles', function() { | ||
| this.add('Articles_enabled', defaults.enable, { |
There was a problem hiding this comment.
Articles_enabled > Articles_Enabled , here and in enableQuery below
app/articles/server/settings.js
Outdated
| public: true, | ||
| }); | ||
|
|
||
| this.add('Articles_admin_panel', 'Articles_admin_panel', { |
There was a problem hiding this comment.
Articles_admin_panel > Articles_Admin_Panel
Also, if we can add some description for this and others above, it would be nice.
client/importPackages.js
Outdated
| @@ -108,3 +108,4 @@ import '../app/ui-cached-collection'; | |||
| import '../app/action-links'; | |||
| import '../app/reactions/client'; | |||
| import '../app/livechat/client'; | |||
| import '../app/articles/client'; | |||
There was a problem hiding this comment.
Can we remove it as we are not using it?
| @@ -111,6 +111,24 @@ export const getActions = ({ user, directActions, hideAdminControls }) => { | |||
| }, | |||
| }, | |||
|
|
|||
| { | |||
| icon: 'articles', | |||
There was a problem hiding this comment.
UX question, should we show this button is user's account is not on ghost?
There was a problem hiding this comment.
If we do this, and store and maintain a state in user objects > ghostAccountExists,,.. then this can simply be a link and the corresponding meteor method is not required.
| @@ -33,7 +33,7 @@ const mentionMessage = (rid, { _id, username, name }, message_embedded) => { | |||
| return Messages.insert(welcomeMessage); | |||
| }; | |||
|
|
|||
| const create = ({ prid, pmid, t_name, reply, users }) => { | |||
| const create = ({ prid, pmid, t_name, reply, t, users }) => { | |||
There was a problem hiding this comment.
Discuss with the team and re-evaluate, weather to implement comments this way.
(allowing public child of a private parent room)
|
|
||
| const api = new API(); | ||
|
|
||
| export const triggerHandler = new class ArticlesSettingsHandler { |
There was a problem hiding this comment.
Refactor it to use ghostClient
| enableCors: true, | ||
| apiPath: 'ghooks/', | ||
| auth: { | ||
| user() { |
There was a problem hiding this comment.
Add rate-limiting, to prevent brute force
There was a problem hiding this comment.
Integrations also don't have any rate-limit, should I add it here. 🤔
| import { settings } from '../../../settings'; | ||
| import * as Models from '../../../models'; | ||
|
|
||
| const Api = new Restivus({ |
There was a problem hiding this comment.
Note: Revisit the security
There was a problem hiding this comment.
Apart from token checking, what else I have to check?
|
Changed as requested. @kb0304 |
This PR adds:
ff9eea8
NOTE: This PR contains commits from all the PRs, merge this after reviewing all the PRs.