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.

Sign up for GitHub

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

Created a locked tag which can be added to notes by mods / admin #9709

Merged
merged 4 commits into from
Jun 3, 2021
Merged

Created a locked tag which can be added to notes by mods / admin #9709

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdabbac
adding locked tag
Manasa2850 May 31, 2021
0eab1d1
normal users can't delete locked tag
Manasa2850 May 31, 2021
52a981b
changed error message
Manasa2850 Jun 1, 2021
58e59b8
added javascript function for locking
Manasa2850 Jun 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/controllers/notes_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,12 @@ def preview
render template: 'notes/show'
end

def lock_note
@node = Node.find(params[:id])
@node.add_tag('locked', current_user)
redirect_to URI.parse(@node.path).path
end

def edit
@node = Node.find_by(nid: params[:id], type: 'note')

Expand Down
5 changes: 4 additions & 1 deletion app/controllers/tag_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ def delete
node_tag = NodeTag.where(nid: params[:nid], tid: params[:tid]).first
node = Node.where(nid: params[:nid]).first
# only admins, mods, and tag authors can delete other peoples' tags
if node_tag.uid == current_user.uid || logged_in_as(['admin', 'moderator']) || node.uid == current_user.uid
if node_tag.uid == current_user.uid || logged_in_as(['admin', 'moderator']) || (node.uid == current_user.uid && node_tag.name != "locked")
Copy link
Contributor

Choose a reason for hiding this comment

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

From line 359 above, we have the node_tag variable derived from this query node_tag = NodeTag.where(nid: params[:nid], tid: params[:tid]).first ,

I am curious as to whether checking to see if node_tag.name != "locked" will work if the first tag derived is not "locked" but contains a list of tags like [air-quality, locked]....the node itself does contain a locked tag but it may not be the first to be picked hence someone else might delete a tag.

Should checking if the tags returned contain "locked", before picking the first one in line 359 help?

Copy link
Contributor

Choose a reason for hiding this comment

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

lol I was just reviewing this and panicked that I had merged it @jywarren 😅

Are the concerns I have above okay? Or will the tagging work as expected... @jywarren?

Copy link
Member

Choose a reason for hiding this comment

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

Oh you're totally right, my bad! Yes, let's change this, we can use @node.has_tag('locked') instead!

Copy link
Member

Choose a reason for hiding this comment

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

@Manasa2850 what do you say? Thanks all!


tag = Tag.joins(:node_tag)
.select('term_data.name')
Expand All @@ -385,6 +385,9 @@ def delete
end
end
end
elsif node_tag.name == "locked"
flash[:error] = "Only admins can delete the locked tag."
redirect_to Node.find_by(nid: params[:nid]).path
else
flash[:error] = I18n.t('tag_controller.must_own_tag_to_delete')
redirect_to Node.find_by(nid: params[:nid]).path
Expand Down
4 changes: 4 additions & 0 deletions app/views/notes/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@
</table>
<% end %>

<% if current_user && ( current_user.role == "admin" || current_user.role == "moderator") && !(@node.has_tag('locked')) %>
Copy link
Member

Choose a reason for hiding this comment

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

What if, in follow up, we moved this button into the advanced tagging menu? This would then appear on wikis too!

<%= link_to "Lock Note", "/notes/lock_note/#{@node.id}" %>
<% end %>

<% if current_user &&
( current_user.role == "admin" || current_user.role == "moderator" ||
current_user.id == @node.id || current_user.is_coauthor?(@node) ) &&
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@
get 'admin/publish_comment/:id' => 'admin#publish_comment'
get 'admin/mark_comment_spam/:id' => 'admin#mark_comment_spam'
get 'smtp_test' => 'admin#smtp_test'
get 'notes/lock_note/:id' => 'notes#lock_note'

get 'post' => 'editor#post', :as => :editor_post
post 'post' => 'editor#post', :as => :editor_path
Expand Down