Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set default tag values for categories install data and tag findMatches #19

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

eunilee2
Copy link
Contributor

Set default values ["exam", "quiz", "homework"] for tagWhitelist variable in the findMatches function for src/topics/tags.js. Also set default tagWhitelist values for categories data upon installation. Placed tagging privileges from registered users to moderator privileges in 'src/categories/create.js'.

By limiting the default privileges for registered users, students are restricted from write access privileges for tagging topics. Setting default values for the tagWhitelist variables in the install data and findMatches functions allows the admin to configure tags across categories and topics easily.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to get this changes to display through running ./nodebb setup again? I know there were some issues with that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! the changes are working locally now 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note for later: since all of our changes hinge on fully uninstalling and reconfiguring NodeBB from default, we should keep track in the README of all the files and commands necessary to run in order to test changes for files in install.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also had to modify this to update default privileges; however, I wasn't able to get these to display. How did you approach it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What difference did you observe between topics / categories ? Was updating both necessary ?

@eunilee2 eunilee2 added this to the Sprint 1 milestone Feb 12, 2025
@eunilee2 eunilee2 linked an issue Feb 12, 2025 that may be closed by this pull request
Copy link
Contributor

@html1101 html1101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! My only comments would be for the future (and this is true for my branch as well) documenting the attributes we're adding into these configuration files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note for later: since all of our changes hinge on fully uninstalling and reconfiguring NodeBB from default, we should keep track in the README of all the files and commands necessary to run in order to test changes for files in install.

@@ -6,7 +6,8 @@
"bgColor": "#fda34b",
"color": "#ffffff",
"icon" : "fa-bullhorn",
"order": 1
"order": 1,
"tagWhitelist": ["exam","quiz","homework"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do this as well with my tags, but documenting what these tags do in the README would be helpful for future development.

@@ -17,6 +17,7 @@ const translator = require('../translator');

module.exports = function (Topics) {
Topics.create = async function (data) {
// console.log('the data for topics.create func is ', data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny bit of dead code here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure Default Privileges [still in progress]
3 participants