-
Notifications
You must be signed in to change notification settings - Fork 0
Automated Test: topic-email-management #333
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import ObjectController from "discourse/controllers/object"; | ||
|
|
||
| export default ObjectController.extend({ | ||
|
|
||
| stopNotificiationsText: function() { | ||
| return I18n.t("topic.unsubscribe.stop_notifications", { title: this.get("model.fancyTitle") }); | ||
| }.property("model.fancyTitle"), | ||
|
|
||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import PostStream from "discourse/models/post-stream"; | ||
|
|
||
| export default Discourse.Route.extend({ | ||
| model(params) { | ||
| const topic = this.store.createRecord("topic", { id: params.id }); | ||
| return PostStream.loadTopicView(params.id).then(json => { | ||
| topic.updateFromJson(json); | ||
| return topic; | ||
| }); | ||
| }, | ||
|
|
||
| afterModel(topic) { | ||
| // hide the notification reason text | ||
| topic.set("details.notificationReasonText", null); | ||
| }, | ||
|
|
||
| actions: { | ||
| didTransition() { | ||
| this.controllerFor("application").set("showFooter", true); | ||
| return true; | ||
| } | ||
| } | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| <div class="container"> | ||
| <p> | ||
| {{{stopNotificiationsText}}} | ||
| </p> | ||
| <p> | ||
| {{i18n "topic.unsubscribe.change_notification_state"}} {{topic-notifications-button topic=model}} | ||
| </p> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export default Discourse.View.extend({ | ||
| classNames: ["topic-unsubscribe"] | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,11 +24,12 @@ class TopicsController < ApplicationController | |
| :bulk, | ||
| :reset_new, | ||
| :change_post_owners, | ||
| :bookmark] | ||
| :bookmark, | ||
| :unsubscribe] | ||
|
|
||
| before_filter :consider_user_for_promotion, only: :show | ||
|
|
||
| skip_before_filter :check_xhr, only: [:show, :feed] | ||
| skip_before_filter :check_xhr, only: [:show, :unsubscribe, :feed] | ||
|
|
||
| def id_for_slug | ||
| topic = Topic.find_by(slug: params[:slug].downcase) | ||
|
|
@@ -94,6 +95,26 @@ def show | |
| raise ex | ||
| end | ||
|
|
||
| def unsubscribe | ||
| @topic_view = TopicView.new(params[:topic_id], current_user) | ||
|
|
||
| if slugs_do_not_match || (!request.format.json? && params[:slug].blank?) | ||
| return redirect_to @topic_view.topic.unsubscribe_url, status: 301 | ||
| end | ||
|
|
||
| tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id]) | ||
|
|
||
| if tu.notification_level > TopicUser.notification_levels[:regular] | ||
| tu.notification_level = TopicUser.notification_levels[:regular] | ||
| else | ||
| tu.notification_level = TopicUser.notification_levels[:muted] | ||
| end | ||
|
|
||
| tu.save! | ||
|
Comment on lines
+105
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle missing
Proposed fix- tu = TopicUser.find_by(user_id: current_user.id, topic_id: params[:topic_id])
-
- if tu.notification_level > TopicUser.notification_levels[:regular]
- tu.notification_level = TopicUser.notification_levels[:regular]
- else
- tu.notification_level = TopicUser.notification_levels[:muted]
- end
-
- tu.save!
+ tu = TopicUser.find_or_initialize_by(user_id: current_user.id, topic_id: params[:topic_id])
+ current_level = tu.notification_level || TopicUser.notification_levels[:regular]
+ new_level =
+ if current_level > TopicUser.notification_levels[:regular]
+ TopicUser.notification_levels[:regular]
+ else
+ TopicUser.notification_levels[:muted]
+ end
+
+ TopicUser.change(current_user, params[:topic_id], notification_level: new_level)🤖 Prompt for AI Agents |
||
|
|
||
| perform_show_response | ||
|
Comment on lines
+98
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid state changes via GET in This action mutates notification settings on a GET request, which is CSRF‑prone for logged‑in users. Consider switching to POST with CSRF protection or using a signed token in the URL to permit a safe one‑click flow. 🧰 Tools🪛 RuboCop (1.84.0)[convention] 98-116: Assignment Branch Condition size for (Metrics/AbcSize) 🤖 Prompt for AI Agents |
||
| end | ||
|
|
||
| def wordpress | ||
| params.require(:best) | ||
| params.require(:topic_id) | ||
|
|
@@ -476,6 +497,7 @@ def perform_show_response | |
| format.html do | ||
| @description_meta = @topic_view.topic.excerpt | ||
| store_preloaded("topic_#{@topic_view.topic.id}", MultiJson.dump(topic_view_serializer)) | ||
| render :show | ||
| end | ||
|
|
||
| format.json do | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,31 +1,29 @@ | ||||||||||||||||
| <div id='main' class=<%= classes %>> | ||||||||||||||||
|
|
||||||||||||||||
| <%= render :partial => 'email/post', :locals => {:post => post} %> | ||||||||||||||||
| <%= render partial: 'email/post', locals: { post: post } %> | ||||||||||||||||
| <% if context_posts.present? %> | ||||||||||||||||
| <div class='footer'> | ||||||||||||||||
| %{respond_instructions} | ||||||||||||||||
| </div> | ||||||||||||||||
| <hr> | ||||||||||||||||
| <h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4> | ||||||||||||||||
| <% if context_posts.present? %> | ||||||||||||||||
| <div class='footer'>%{respond_instructions}</div> | ||||||||||||||||
|
|
||||||||||||||||
| <hr> | ||||||||||||||||
|
|
||||||||||||||||
|
Comment on lines
+5
to
9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid duplicate respond instructions when context posts exist.
Proposed fix- <% if context_posts.present? %>
- <div class='footer'>%{respond_instructions}</div>
-
- <hr>
+ <% if context_posts.present? %>
+ <hr>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| <% context_posts.each do |p| %> | ||||||||||||||||
| <%= render :partial => 'email/post', :locals => {:post => p} %> | ||||||||||||||||
| <h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4> | ||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix class attribute typo for the previous‑discussion header.
Proposed fix- <h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
+ <h4 class='previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| <% context_posts.each do |p| %> | ||||||||||||||||
| <%= render partial: 'email/post', locals: { post: p } %> | ||||||||||||||||
| <% end %> | ||||||||||||||||
| <% end %> | ||||||||||||||||
| <% end %> | ||||||||||||||||
|
|
||||||||||||||||
| <hr> | ||||||||||||||||
| <hr> | ||||||||||||||||
|
|
||||||||||||||||
| <div class='footer'>%{respond_instructions}</div> | ||||||||||||||||
| <div class='footer'>%{unsubscribe_link}</div> | ||||||||||||||||
|
|
||||||||||||||||
| <div class='footer'> | ||||||||||||||||
| %{respond_instructions} | ||||||||||||||||
| </div> | ||||||||||||||||
| <div class='footer'> | ||||||||||||||||
| %{unsubscribe_link} | ||||||||||||||||
| </div> | ||||||||||||||||
| </div> | ||||||||||||||||
|
|
||||||||||||||||
| <div itemscope itemtype="http://schema.org/EmailMessage" style="display:none"> | ||||||||||||||||
| <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction"> | ||||||||||||||||
| <link itemprop="url" href="<%= Discourse.base_url %><%= post.url %>" /> | ||||||||||||||||
| <meta itemprop="name" content="<%= t 'read_full_topic' %>"/> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction"> | ||||||||||||||||
| <link itemprop="url" href="<%= Discourse.base_url %><%= post.url %>" /> | ||||||||||||||||
| <meta itemprop="name" content="<%= t 'read_full_topic' %>"/> | ||||||||||||||||
| </div> | ||||||||||||||||
| </div> | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 477
🏁 Script executed:
rg -in 'stopNotifi' app/ --type-listRepository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 477
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 222
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 298
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 70
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 950
🏁 Script executed:
Repository: admin-coderabbit/coderabbit-discourse-mirror
Length of output: 259
Typo in computed property name:
stopNotificiationsText→stopNotificationsText.The property name has an extra
i—Notificiationsinstead ofNotifications. Although the template also uses the same misspelled name (so the binding works), this creates confusion for any future developer and doesn't match the actual English word. Fix both the property definition and template reference.✏️ Proposed fix
Also update
app/assets/javascripts/discourse/templates/topic/unsubscribe.hbsline 3:📝 Committable suggestion
🤖 Prompt for AI Agents