-
Notifications
You must be signed in to change notification settings - Fork 0
FEATURE: per-topic unsubscribe option in emails #9
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
base: email-notifications-enhancement
Are you sure you want to change the base?
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; | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+10
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. Add error handling for topic loading. The Apply this diff to add error handling: model(params) {
const topic = this.store.createRecord("topic", { id: params.id });
- return PostStream.loadTopicView(params.id).then(json => {
- topic.updateFromJson(json);
- return topic;
- });
+ return PostStream.loadTopicView(params.id)
+ .then(json => {
+ topic.updateFromJson(json);
+ return topic;
+ })
+ .catch(() => {
+ this.transitionTo("discovery.latest");
+ });
},📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 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}}} | ||
|
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. 🧩 Analysis chainTypo in variable name: "stopNotificiationsText". The variable is misspelled as Additionally, the triple braces Apply this diff if the typo needs correction: - {{{stopNotificiationsText}}}
+ {{{stopNotificationsText}}}🏁 Script executed: #!/bin/bash
# Verify the exact content of the template file
cat app/assets/javascripts/discourse/templates/topic/unsubscribe.hbsLength of output: 191 🏁 Script executed: #!/bin/bash
# Search for stopNotifications variable in controller and helper files
rg -n "stopNotif" --type=ts --type=js -C3 app/Length of output: 0 🏁 Script executed: #!/bin/bash
# Search for this specific template being rendered or referenced
rg -n "unsubscribe" --type=ts --type=js app/ | head -20Length of output: 0 🏁 Script executed: #!/bin/bash
# Search for stopNotifications (correct spelling) anywhere in codebase
rg -n "stopNotifications" --type-list | head -5
rg -n "stopNotifications" -C3Length of output: 389 🏁 Script executed: #!/bin/bash
# Search for stopNotificiations (exact typo) everywhere
rg -n "stopNotificiations" -C3Length of output: 1303 🏁 Script executed: #!/bin/bash
# Find controller/route files related to unsubscribe
fd -e "js" -e "ts" -e "rb" | xargs grep -l "unsubscribe" 2>/dev/null | head -20Length of output: 557 🏁 Script executed: #!/bin/bash
# Look for Ember controller files
fd -name "*controller*" -o -name "*route*" | head -20Length of output: 230 Typo confirmed in variable name across multiple files: "stopNotificiationsText" should be "stopNotificationsText". The misspelling exists consistently in both the template and controller, so the code functionally works. However, this spelling error should be corrected for maintainability:
The triple braces in the template render unescaped HTML; verify the controller's computed property sanitizes the content appropriately. 🤖 Prompt for AI Agents |
||
| </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 nil TopicUser record. The code calls Apply this diff to handle the nil case: 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!
+ if tu
+ 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!
+ else
+ TopicUser.create!(
+ user_id: current_user.id,
+ topic_id: params[:topic_id],
+ notification_level: TopicUser.notification_levels[:muted]
+ )
+ end📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| perform_show_response | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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> | ||
|
|
||
| <% 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. Remove the leading dot from the class name. The class attribute contains Apply this diff to fix the class name: - <h4 class='.previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>
+ <h4 class='previous-discussion'><%= t "user_notifications.previous_discussion" %></h4>🤖 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.
Fix typo in method name.
The method name
stopNotificiationsTexthas a typo: "Notificiations" should be "Notifications". This will cause the template to fail when trying to access this property.Apply this diff to fix the typo:
📝 Committable suggestion
🤖 Prompt for AI Agents