From 83e74e80064324d99927d85138e8b230ef485db5 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Sun, 7 Jan 2024 19:36:55 +0100
Subject: [PATCH 01/31] Add missing "s" to inverse of (#582)
---
app/models/quiz_certificate.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/models/quiz_certificate.rb b/app/models/quiz_certificate.rb
index e73fd7b6f..80c9fa8ed 100644
--- a/app/models/quiz_certificate.rb
+++ b/app/models/quiz_certificate.rb
@@ -1,5 +1,5 @@
class QuizCertificate < ApplicationRecord
- belongs_to :quiz, class_name: "Medium", inverse_of: :quiz_certificate
+ belongs_to :quiz, class_name: "Medium", inverse_of: :quiz_certificates
belongs_to :user, optional: true
before_create :set_code
From c769834ddf436e11bede49621ebc45c71701b3cf Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Mon, 8 Jan 2024 17:25:29 +0100
Subject: [PATCH 02/31] Fix missing `to_a` to convert to Rails array (#584)
---
app/controllers/media_controller.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb
index cfb78dd6e..23fd99052 100644
--- a/app/controllers/media_controller.rb
+++ b/app/controllers/media_controller.rb
@@ -636,7 +636,7 @@ def search_results
unless current_user.admin || @lecture.edited_by?(current_user)
lecture_tags = @lecture.tags_including_media_tags
search_results.reject! do |m|
- m.teachable_type == "Course" && !m.tags.intersect?(lecture_tags)
+ m.teachable_type == "Course" && !m.tags.to_a.intersect?(lecture_tags)
end
end
end
From 2a89bf33d51d1d12b7f752bd7db306a73700248a Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Thu, 11 Jan 2024 19:05:36 +0100
Subject: [PATCH 03/31] Warn about too long GitHub commit messages (#586)
---
.vscode/settings.json | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/.vscode/settings.json b/.vscode/settings.json
index 21983ff1e..24b00c4b5 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -79,6 +79,12 @@
}
],
//////////////////////////////////////
+ // Git
+ //////////////////////////////////////
+ "git.inputValidation": "warn",
+ "git.inputValidationSubjectLength": 50,
+ "git.inputValidationLength": 72,
+ //////////////////////////////////////
// Spell Checker
//////////////////////////////////////
"cSpell.words": [
From 61b114cc284fd7d396578df0af48e620d0da4a83 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Tue, 16 Jan 2024 19:15:51 +0100
Subject: [PATCH 04/31] Fix comment status (#585)
* Reapply first fix for Reader/Media
See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.
* Correctly show latest post (might be current_user's comment)
* Fix update of unread comments logic in comments controller
* Fix update icon logic and latest post comment
* Simplify latest comment logic
* Improve code comments
* Further improve comments
* Fix wording in comment
* Fix construction of media array & use `.blank?` instead of `.empty?`
---
.../commontator/comments_controller.rb | 51 +++++++++++++------
app/controllers/main_controller.rb | 2 +-
app/controllers/readers_controller.rb | 2 +-
app/models/user.rb | 38 +++++++++++---
app/views/main/comments.html.erb | 4 +-
5 files changed, 70 insertions(+), 27 deletions(-)
diff --git a/app/controllers/commontator/comments_controller.rb b/app/controllers/commontator/comments_controller.rb
index 31058b3da..331919345 100644
--- a/app/controllers/commontator/comments_controller.rb
+++ b/app/controllers/commontator/comments_controller.rb
@@ -71,6 +71,7 @@ def create
Commontator::Subscription.comment_created(@comment)
# The next line constitutes a customization of the original controller
update_unread_status
+ activate_unread_comments_icon_if_necessary
@commontator_page = @commontator_thread.new_comment_page(
@comment.parent_id, @commontator_show_all
@@ -194,9 +195,14 @@ def subscribe_mentioned
end
end
- # This method ensures that the unread_comments flag is updated
- # for users affected by the creation of a newly created comment
- # It constitues a customization
+ # Updates the unread_comments flag for users subscribed to the current thread.
+ # This method should only be called when a new comment was created.
+ #
+ # The originator of the comment does not get the flag set since that user
+ # already knows about the comment; that user has just created it after all.
+ #
+ # (This is a customization of the original controller provided
+ # by the commontator gem.)
def update_unread_status
medium = @commontator_thread.commontable
return unless medium.released.in?(["all", "users", "subscribers"])
@@ -205,22 +211,35 @@ def update_unread_status
relevant_users.where.not(id: current_user.id)
.where(unread_comments: false)
.update(unread_comments: true)
-
- # make sure that the thread associated to this comment is marked as read
- # by the comment creator (unless some other user posted a comment in it
- # that has not yet been read)
- @reader = Reader.find_or_create_by(user: current_user,
- thread: @commontator_thread)
- if unseen_comments?
- @update_icon = true
- return
- end
- @reader.touch
end
- def unseen_comments?
+ # Might activate the flag used in the view to indicate unread comments.
+ # This method should only be called when a new comment was created.
+ # The flag is activated if the current user has not seen all comments
+ # in the thread in which the new comment was created.
+ #
+ # The flag might only be activated, not deactivated since the checks
+ # performed here are not sufficient to determine whether a user has
+ # any unread comments (including those in possibly other threads).
+ #
+ # This method was introduced for one specific edge case:
+ # When the current user A has just created a new comment in a thread,
+ # but in the meantime, another user B has created a comment in the same
+ # thread. User A will not be informed immediately about the new comment
+ # by B since we don't have websockets implemented. Instead, A will be
+ # informed by a visual indicator as soon as A has posted their own comment.
+ #
+ # (This is a customization of the original controller provided
+ # by the commontator gem.)
+ def activate_unread_comments_icon_if_necessary
+ reader = Reader.find_by(user: current_user, thread: @commontator_thread)
+ @update_icon = true if unseen_comments_in_current_thread?(reader)
+ end
+
+ def unseen_comments_in_current_thread?(reader = nil)
@commontator_thread.comments.any? do |c|
- c.creator != current_user && c.created_at > @reader.updated_at
+ not_marked_as_read_in_reader = reader.nil? || c.created_at > reader.updated_at
+ c.creator != current_user && not_marked_as_read_in_reader
end
end
end
diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb
index 8fa45237c..c7e8d469b 100644
--- a/app/controllers/main_controller.rb
+++ b/app/controllers/main_controller.rb
@@ -26,7 +26,7 @@ def sponsors
end
def comments
- @media_comments = current_user.media_latest_comments
+ @media_comments = current_user.subscribed_media_with_latest_comments_not_by_creator
@media_comments.select! do |m|
(Reader.find_by(user: current_user, thread: m[:thread])
&.updated_at || 1000.years.ago) < m[:latest_comment].created_at &&
diff --git a/app/controllers/readers_controller.rb b/app/controllers/readers_controller.rb
index f276f2989..931b93a45 100644
--- a/app/controllers/readers_controller.rb
+++ b/app/controllers/readers_controller.rb
@@ -9,7 +9,7 @@ def update
@reader = Reader.find_or_create_by(user: current_user,
thread: @thread)
@reader.touch
- @anything_left = current_user.media_latest_comments.any? do |m|
+ @anything_left = current_user.subscribed_media_with_latest_comments_not_by_creator.any? do |m|
(Reader.find_by(user: current_user, thread: m[:thread])
&.updated_at || 1000.years.ago) < m[:latest_comment].created_at
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 7745f2260..40feb6f60 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -549,14 +549,38 @@ def subscribed_commentable_media_with_comments
.select { |m| m.commontator_thread.comments.any? }
end
- def media_latest_comments
- media = subscribed_commentable_media_with_comments
- .map do |m|
- { medium: m,
- thread: m.commontator_thread,
- latest_comment: m.commontator_thread
- .comments.max_by(&:created_at) }
+ # Returns the media that the user has subscribed to and that have been
+ # commented on by somebody else (not by the current user). The order is
+ # given by the time of the latest comment by somebody else.
+ #
+ # Media that have not been commented on by somebody else than the current user,
+ # are not returned (!).
+ #
+ # For each medium, the following information is stored:
+ # - the medium itself
+ # - the thread of the medium
+ # - the latest comment by somebody else than the current user
+ # - the latest comment by any user (which might include the current user)
+ def subscribed_media_with_latest_comments_not_by_creator
+ media = []
+
+ subscribed_commentable_media_with_comments.each do |m|
+ thread = m.commontator_thread
+ comments = thread.comments
+ next if comments.blank?
+
+ comments_not_by_creator = comments.reject { |c| c.creator == self }
+ next if comments_not_by_creator.blank?
+
+ latest_comment = comments_not_by_creator.max_by(&:created_at)
+ latest_comment_by_any_user = comments.max_by(&:created_at)
+
+ media << { medium: m,
+ thread: thread,
+ latest_comment: latest_comment,
+ latest_comment_by_any_user: latest_comment_by_any_user }
end
+
media.sort_by { |x| x[:latest_comment].created_at }.reverse
end
diff --git a/app/views/main/comments.html.erb b/app/views/main/comments.html.erb
index 13e122bad..22f6e5ad4 100644
--- a/app/views/main/comments.html.erb
+++ b/app/views/main/comments.html.erb
@@ -68,8 +68,8 @@
From cfae7141a395d210f8c4b6e03a1eddb7a0f5165f Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Wed, 17 Jan 2024 23:21:59 +0100
Subject: [PATCH 05/31] Migrate `unread_comments` flag (fix inconsistencies)
(#587)
* Add dummy migration
* Implement migration for unread comment flag
* Remove unnecessary comment
* Declare migration as not idempotent
* Use array.length instead of counting
* Throw error to prevent revert of migration
* Fix severe flaws in unread comments migration
* Simplify Reader retrieval
* Use the more explicit `.nil?` method
* Update migration date
* Fix annoying bug: don't use `.select!` but `.select`
* Polish migration
e.g. update comment, more suitable name for the method etc.
* Rename method according to #585
---
...000_fix_unread_comments_inconsistencies.rb | 56 +++++++++++++++++++
db/schema.rb | 2 +-
2 files changed, 57 insertions(+), 1 deletion(-)
create mode 100644 db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
diff --git a/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
new file mode 100644
index 000000000..3392da3dc
--- /dev/null
+++ b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
@@ -0,0 +1,56 @@
+# Fixes the unread_comments flag for all users. Unintended behavior was
+# introduced in pull request #515. Migration introduced in #587.
+# Behavior fixed in #585.
+#
+# This migration is generally *not* idempotent since users might have interacted
+# with the website since the migration was run and thus they will probably have
+# different unread comments flags as the ones at the time of the migration.
+#
+# This migration is not reversible as we don't store the previous state of
+# the unread_comments flag.
+class FixUnreadCommentsInconsistencies < ActiveRecord::Migration[7.0]
+ def up
+ num_fixed_users = 0
+
+ User.find_each do |user|
+ had_user_unread_comments = user.unread_comments # boolean
+ has_user_unread_comments = user_unread_comments?(user)
+
+ has_flag_changed = (had_user_unread_comments != has_user_unread_comments)
+ user.update(unread_comments: has_user_unread_comments) if has_flag_changed
+ num_fixed_users += 1 if has_flag_changed
+ end
+
+ Rails.logger.debug { "Ran through #{User.count} users (unread comments flag)" }
+ Rails.logger.debug { "Fixed #{num_fixed_users} users (unread comments flag)" }
+ end
+
+ # Checks and returns whether the user has unread comments.
+ def user_unread_comments?(user)
+ # Check for unread comments -- directly via Reader
+ readers = Reader.where(user: user)
+ readers.each do |reader|
+ thread = Commontator::Thread.find_by(id: reader.thread_id)
+ next if thread.nil?
+
+ latest_thread_comment_by_any_user = thread.comments.max_by(&:created_at)
+ next if latest_thread_comment_by_any_user.blank?
+
+ latest_thread_comment_time = latest_thread_comment_by_any_user.created_at
+ has_user_unread_comments = reader.updated_at < latest_thread_comment_time
+
+ return true if has_user_unread_comments
+ end
+
+ # User might still have unread comments but no related Reader objects
+ # -> Check for unread comments -- via Media
+ unseen_media = user.subscribed_media_with_latest_comments_not_by_creator.select do |m|
+ m[:medium].visible_for_user?(user)
+ end
+ unseen_media.present?
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 923aed6fb..0eb8c6842 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2023_11_01_100015) do
+ActiveRecord::Schema[7.0].define(version: 2024_01_16_180000) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
From 798db61c1ddae7fbbcd37a3966386ff0fe395055 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Fri, 19 Jan 2024 16:06:36 +0100
Subject: [PATCH 06/31] Use `warn` log level for migration (#588)
---
.../20240116180000_fix_unread_comments_inconsistencies.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
index 3392da3dc..74ef88755 100644
--- a/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
+++ b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
@@ -21,8 +21,8 @@ def up
num_fixed_users += 1 if has_flag_changed
end
- Rails.logger.debug { "Ran through #{User.count} users (unread comments flag)" }
- Rails.logger.debug { "Fixed #{num_fixed_users} users (unread comments flag)" }
+ Rails.logger.warn { "Ran through #{User.count} users (unread comments flag)" }
+ Rails.logger.warn { "Fixed #{num_fixed_users} users (unread comments flag)" }
end
# Checks and returns whether the user has unread comments.
From b3b7f6ac965e07722c48299aa5f18359ac20db70 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Mon, 12 Feb 2024 23:24:36 +0100
Subject: [PATCH 07/31] Rename get_statistics.coffee file to statistics.coffee
(#591)
This is to reflect the corresponding renaming of the route
due to rubocop.
---
app/views/media/{get_statistics.coffee => statistics.coffee} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename app/views/media/{get_statistics.coffee => statistics.coffee} (100%)
diff --git a/app/views/media/get_statistics.coffee b/app/views/media/statistics.coffee
similarity index 100%
rename from app/views/media/get_statistics.coffee
rename to app/views/media/statistics.coffee
From 7e7d49d60f0c15f5699037206cb50ee065750adb Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Thu, 15 Feb 2024 16:53:25 +0100
Subject: [PATCH 08/31] Fix `unread_comments` migration (#594)
* Add fix for migration #587
* Mark flawed unread comments migration as stale
---
...000_fix_unread_comments_inconsistencies.rb | 101 ++++++++++++------
...x_unread_comments_inconsistencies_again.rb | 45 ++++++++
db/schema.rb | 2 +-
3 files changed, 114 insertions(+), 34 deletions(-)
create mode 100644 db/migrate/20240215100000_fix_unread_comments_inconsistencies_again.rb
diff --git a/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
index 74ef88755..9d1a0eb19 100644
--- a/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
+++ b/db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb
@@ -10,45 +10,80 @@
# the unread_comments flag.
class FixUnreadCommentsInconsistencies < ActiveRecord::Migration[7.0]
def up
- num_fixed_users = 0
+ # 🛑 This migration contains a bug, please don't use it again. Instead,
+ # refer to the migration in 20240215100000_fix_unread_comments_inconsistencies_again.rb.
+ #
+ # 💡 Explanation of the bug:
+ # The bug resides within the method `user_unread_comments?`. It was supposed
+ # to reflect the logic of the method `comments` in app/controllers/main_controller.rb:
+ #
+ # def comments
+ # @media_comments = current_user.subscribed_media_with_latest_comments_not_by_creator
+ # @media_comments.select! do |m|
+ # (Reader.find_by(user: current_user, thread: m[:thread])
+ # &.updated_at || 1000.years.ago) < m[:latest_comment].created_at &&
+ # m[:medium].visible_for_user?(current_user)
+ # end
+ # @media_array = Kaminari.paginate_array(@media_comments)
+ # .page(params[:page]).per(10)
+ # end
+ #
+ # The method `user_unread_comments?` in this migration, however, does not
+ # reflect the logic of the method `comments` in app/controllers/main_controller.rb
+ # precisely. Consider what happens when for each reader instance, the
+ # check reader.updated_at < latest_thread_comment_time is false. In this case,
+ # we don't encounter an early "return true" and therefore go on with the second
+ # part. There, we want to check if there are any *unseen* media. What we actually do
+ # is to check if there are any media with comments (that are not by the creator
+ # and that are visible to the user). But we missed the part where we check if
+ # the user has already seen these comments, i.e. an additional reader query
+ # is missing. This is why suddenly, many more people encounter the original
+ # issue after having run this migration.
+ #
+ # This migration took ~40 minutes to run for ~7000 users at 2024-02-15, 0:40.
+ raise ActiveRecord::IrreversibleMigration
+
+ # For archive reasons, here is the original code:
- User.find_each do |user|
- had_user_unread_comments = user.unread_comments # boolean
- has_user_unread_comments = user_unread_comments?(user)
+ # num_fixed_users = 0
- has_flag_changed = (had_user_unread_comments != has_user_unread_comments)
- user.update(unread_comments: has_user_unread_comments) if has_flag_changed
- num_fixed_users += 1 if has_flag_changed
- end
+ # User.find_each do |user|
+ # had_user_unread_comments = user.unread_comments # boolean
+ # has_user_unread_comments = user_unread_comments?(user)
- Rails.logger.warn { "Ran through #{User.count} users (unread comments flag)" }
- Rails.logger.warn { "Fixed #{num_fixed_users} users (unread comments flag)" }
+ # has_flag_changed = (had_user_unread_comments != has_user_unread_comments)
+ # user.update(unread_comments: has_user_unread_comments) if has_flag_changed
+ # num_fixed_users += 1 if has_flag_changed
+ # end
+
+ # Rails.logger.warn { "Ran through #{User.count} users (unread comments flag)" }
+ # Rails.logger.warn { "Fixed #{num_fixed_users} users (unread comments flag)" }
end
# Checks and returns whether the user has unread comments.
- def user_unread_comments?(user)
- # Check for unread comments -- directly via Reader
- readers = Reader.where(user: user)
- readers.each do |reader|
- thread = Commontator::Thread.find_by(id: reader.thread_id)
- next if thread.nil?
-
- latest_thread_comment_by_any_user = thread.comments.max_by(&:created_at)
- next if latest_thread_comment_by_any_user.blank?
-
- latest_thread_comment_time = latest_thread_comment_by_any_user.created_at
- has_user_unread_comments = reader.updated_at < latest_thread_comment_time
-
- return true if has_user_unread_comments
- end
-
- # User might still have unread comments but no related Reader objects
- # -> Check for unread comments -- via Media
- unseen_media = user.subscribed_media_with_latest_comments_not_by_creator.select do |m|
- m[:medium].visible_for_user?(user)
- end
- unseen_media.present?
- end
+ # def user_unread_comments?(user)
+ # # Check for unread comments -- directly via Reader
+ # readers = Reader.where(user: user)
+ # readers.each do |reader|
+ # thread = Commontator::Thread.find_by(id: reader.thread_id)
+ # next if thread.nil?
+
+ # latest_thread_comment_by_any_user = thread.comments.max_by(&:created_at)
+ # next if latest_thread_comment_by_any_user.blank?
+
+ # latest_thread_comment_time = latest_thread_comment_by_any_user.created_at
+ # has_user_unread_comments = reader.updated_at < latest_thread_comment_time
+
+ # return true if has_user_unread_comments
+ # end
+
+ # # User might still have unread comments but no related Reader objects
+ # # -> Check for unread comments -- via Media
+ # unseen_media = user.subscribed_media_with_latest_comments_not_by_creator.select do |m|
+ # m[:medium].visible_for_user?(user)
+ # end
+ # unseen_media.present?
+ # end
def down
raise ActiveRecord::IrreversibleMigration
diff --git a/db/migrate/20240215100000_fix_unread_comments_inconsistencies_again.rb b/db/migrate/20240215100000_fix_unread_comments_inconsistencies_again.rb
new file mode 100644
index 000000000..02420a74d
--- /dev/null
+++ b/db/migrate/20240215100000_fix_unread_comments_inconsistencies_again.rb
@@ -0,0 +1,45 @@
+# Fixes the unread_comments flag for all users. Unintended behavior was
+# introduced in pull request #515. Behavior fixed in #585
+# A migration was introduced in #587, but it turned out it contained a bug.
+# This migration here is a fix for the migration in #587.
+#
+# This migration is generally *not* idempotent since users might have interacted
+# with the website since the migration was run and thus they will probably have
+# different unread comments flags as the ones at the time of the migration.
+#
+# This migration is not reversible as we don't store the previous state of
+# the unread_comments flag.
+class FixUnreadCommentsInconsistenciesAgain < ActiveRecord::Migration[7.0]
+ def up
+ num_fixed_users = 0
+
+ User.find_each do |user|
+ had_user_unread_comments = user.unread_comments # boolean
+ has_user_unread_comments = user_unread_comments?(user)
+
+ has_flag_changed = (had_user_unread_comments != has_user_unread_comments)
+ user.update(unread_comments: has_user_unread_comments) if has_flag_changed
+ num_fixed_users += 1 if has_flag_changed
+ end
+
+ Rails.logger.warn { "Ran through #{User.count} users (unread comments flag again)" }
+ Rails.logger.warn { "Fixed #{num_fixed_users} users (unread comments flag again)" }
+ end
+
+ # Checks and returns whether the user has unread comments.
+ def user_unread_comments?(user)
+ # see the method "comments" in app/controllers/main_controller.rb
+ unseen_media = user.subscribed_media_with_latest_comments_not_by_creator
+ unseen_media.select! do |m|
+ (Reader.find_by(user: user, thread: m[:thread])
+ &.updated_at || 1000.years.ago) < m[:latest_comment].created_at &&
+ m[:medium].visible_for_user?(user)
+ end
+
+ unseen_media.present?
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0eb8c6842..377a05a4f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2024_01_16_180000) do
+ActiveRecord::Schema[7.0].define(version: 2024_02_15_100000) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
From 95168d8e4409fab3e8eee0a30e20139641efba7a Mon Sep 17 00:00:00 2001
From: fosterfarrell9 <28628554+fosterfarrell9@users.noreply.github.com>
Date: Fri, 16 Feb 2024 11:48:12 +0100
Subject: [PATCH 09/31] Fix erroneous media search (#593)
* Warn about too long GitHub commit messages (#586)
* Fix comment status (#585)
* Reapply first fix for Reader/Media
See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.
* Correctly show latest post (might be current_user's comment)
* Fix update of unread comments logic in comments controller
* Fix update icon logic and latest post comment
* Simplify latest comment logic
* Improve code comments
* Further improve comments
* Fix wording in comment
* Fix construction of media array & use `.blank?` instead of `.empty?`
* Migrate `unread_comments` flag (fix inconsistencies) (#587)
* Add dummy migration
* Implement migration for unread comment flag
* Remove unnecessary comment
* Declare migration as not idempotent
* Use array.length instead of counting
* Throw error to prevent revert of migration
* Fix severe flaws in unread comments migration
* Simplify Reader retrieval
* Use the more explicit `.nil?` method
* Update migration date
* Fix annoying bug: don't use `.select!` but `.select`
* Polish migration
e.g. update comment, more suitable name for the method etc.
* Rename method according to #585
* Use `warn` log level for migration (#588)
* Rename get_statistics.coffee file to statistics.coffee (#591)
This is to reflect the corresponding renaming of the route
due to rubocop.
* fix behaviour of media search
* Disable OR/AND buttons if "all" tags is enabled
(inside the media search, right now only frontend change)
* Use ids autogenerated by Rails
This is such that we can still click on the label to select
the respective radio button, instead of only being able
to click on the radio button itself.
* Disable AND/OR initially (until user deselects "all tags")
* Don't restrict media search if "all" tags is enabled.
If the "all tags" option is enabled in the search, we should allow the
results to include *any* tag. This is automatically the case if we just
don't add any restriction regarding the tag ids in the SOLR search.
This change accompanies the frontend change to disable the AND/OR
buttons if the "all" button is selected meaning the user wants to search
for all tags. See #593 for more details.
---------
Co-authored-by: Splines <37160523+Splines@users.noreply.github.com>
Co-authored-by: Splines
---
app/assets/javascripts/application.js | 1 +
app/assets/javascripts/search_tags.js | 19 +++++++++++++++++++
app/controllers/media_controller.rb | 15 ---------------
app/models/medium.rb | 16 +++++-----------
app/views/main/start/_media_search.html.erb | 6 ++++--
app/views/media/catalog/_search_form.html.erb | 6 ++++--
6 files changed, 33 insertions(+), 30 deletions(-)
create mode 100644 app/assets/javascripts/search_tags.js
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 9834213e1..31511217e 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -57,3 +57,4 @@
//= require vertices
//= require watchlists
//= require turbolinks
+//= require search_tags
\ No newline at end of file
diff --git a/app/assets/javascripts/search_tags.js b/app/assets/javascripts/search_tags.js
new file mode 100644
index 000000000..2aa798305
--- /dev/null
+++ b/app/assets/javascripts/search_tags.js
@@ -0,0 +1,19 @@
+$(document).on("turbolinks:load", function () {
+ $("#search_all_tags").change(evt => toggleSearchAllTags(evt));
+});
+
+/**
+ * Dynamically enable/disable the OR/AND buttons in the media search form.
+ * If the user has decided to search for media regardless of the tags,
+ * i.e. they enable the "all" (tags) button, we disable the "OR/AND" buttons
+ * as it is pointless to search for media that references *all* available tags
+ * at once.
+ */
+function toggleSearchAllTags(evt) {
+ const searchAllTags = evt.target.checked;
+ if (searchAllTags) {
+ $("#search_tag_operator_or").prop("checked", true);
+ }
+ $("#search_tag_operator_or").prop("disabled", searchAllTags);
+ $("#search_tag_operator_and").prop("disabled", searchAllTags);
+}
diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb
index 23fd99052..a8efdbd2c 100644
--- a/app/controllers/media_controller.rb
+++ b/app/controllers/media_controller.rb
@@ -228,21 +228,6 @@ def search
results = search.results
@total = search.total
- # in the case of a search with tag_operator 'or', we
- # execute two searches and merge the results, where media
- # with the selected tags are now shown at the front of the list
- if (search_params[:tag_operator] == "or") \
- && (search_params[:all_tags] == "0") \
- && (search_params[:fulltext].size >= 2)
- params["search"]["all_tags"] = "1"
- search_no_tags = Medium.search_by(search_params, params[:page])
- search_no_tags.execute
- results_no_tags = search_no_tags.results
- results = (results + results_no_tags).uniq
- @total = results.size
- params["search"]["all_tags"] = "0"
- end
-
if filter_media
search_arel = Medium.where(id: results.pluck(:id))
visible_search_results = current_user.filter_visible_media(search_arel)
diff --git a/app/models/medium.rb b/app/models/medium.rb
index 0fe6faaa5..645371915 100644
--- a/app/models/medium.rb
+++ b/app/models/medium.rb
@@ -301,9 +301,6 @@ def self.search_by(search_params, _page)
search_params[:all_terms] = "1" if search_params[:all_terms].blank?
search_params[:all_teachers] = "1" if search_params[:all_teachers].blank?
search_params[:term_ids].push("0") if search_params[:term_ids].present?
- if search_params[:all_tags] == "1" && search_params[:tag_operator] == "and"
- search_params[:tag_ids] = Tag.pluck(:id)
- end
user = User.find_by(id: search_params[:user_id])
search = Sunspot.new_search(Medium)
search.build do
@@ -336,15 +333,12 @@ def self.search_by(search_params, _page)
with(:release_state, search_params[:access])
end
end
- if !search_params[:all_tags] == "1" &&
- !search_params[:tag_operator] == "or" && (search_params[:tag_ids])
- if search_params[:tag_operator] == "or" || search_params[:all_tags] == "1"
- search.build do
- with(:tag_ids).any_of(search_params[:tag_ids])
- end
- else
- search.build do
+ if search_params[:all_tags] == "0" && search_params[:tag_ids].any?
+ search.build do
+ if search_params[:tag_operator] == "and"
with(:tag_ids).all_of(search_params[:tag_ids])
+ else
+ with(:tag_ids).any_of(search_params[:tag_ids])
end
end
end
diff --git a/app/views/main/start/_media_search.html.erb b/app/views/main/start/_media_search.html.erb
index b836981b2..2a0f2dcfa 100644
--- a/app/views/main/start/_media_search.html.erb
+++ b/app/views/main/start/_media_search.html.erb
@@ -62,7 +62,8 @@
<%= f.radio_button :tag_operator,
'or',
checked: true,
- class: 'form-check-input' %>
+ class: 'form-check-input',
+ disabled: true %>
<%= f.label :tag_operator,
t('basics.OR'),
value: 'or',
@@ -71,7 +72,8 @@
<% end %>
diff --git a/app/views/exception_handler/mailers/new_exception.html.erb b/app/views/exception_handler/mailers/new_exception.html.erb
index 93b3b9b89..0ebfbbe16 100644
--- a/app/views/exception_handler/mailers/new_exception.html.erb
+++ b/app/views/exception_handler/mailers/new_exception.html.erb
@@ -1,5 +1,5 @@
<%= t('exception.exception_report',
- host: ENV['URL_HOST']) %>
+ host: ENV.fetch("URL_HOST")) %>
<%= @exception.response %> (<%= @exception.status %>)
diff --git a/app/workers/interaction_saver.rb b/app/workers/interaction_saver.rb
index f6c95faf1..344d53830 100644
--- a/app/workers/interaction_saver.rb
+++ b/app/workers/interaction_saver.rb
@@ -2,8 +2,8 @@ class InteractionSaver
include Sidekiq::Worker
def perform(session_id, full_path, referrer, study_participant)
- referrer_url = if referrer.to_s.include?(ENV["URL_HOST"])
- referrer.to_s.remove(ENV["URL_HOST"])
+ referrer_url = if referrer.to_s.include?(ENV.fetch("URL_HOST"))
+ referrer.to_s.remove(ENV.fetch("URL_HOST"))
.remove("https://").remove("http://")
end
Interaction.create(session_id: Digest::SHA2.hexdigest(session_id).first(10),
diff --git a/config/application.rb b/config/application.rb
index 15040fd01..b5e4e0f1e 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -24,7 +24,7 @@ class Application < Rails::Application
# the framework and any gems in your application.
config.exception_handler = {
# sends exception emails to a listed email (string // "you@email.com")
- email: ENV.fetch("ERROR_EMAIL", nil),
+ email: ENV.fetch("ERROR_EMAIL"),
# All keys interpolated as strings, so you can use
# symbols, strings or integers where necessary
diff --git a/config/environments/production.rb b/config/environments/production.rb
index 805b5c66e..ec2a4e303 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -55,13 +55,13 @@
config.log_tags = [:request_id]
# Use a different cache store in production.
- config.cache_store = :mem_cache_store, ENV.fetch("MEMCACHED_SERVER", nil)
+ config.cache_store = :mem_cache_store, ENV.fetch("MEMCACHED_SERVER")
# Use a real queuing backend for Active Job (and separate queues per environment)
# config.active_job.queue_adapter = :resque
# config.active_job.queue_name_prefix = "mampf_#{Rails.env}"
config.action_mailer.perform_caching = false
- config.action_mailer.default_url_options = { protocol: "https", host: ENV.fetch("URL_HOST", nil) }
+ config.action_mailer.default_url_options = { protocol: "https", host: ENV.fetch("URL_HOST") }
config.action_mailer.delivery_method = :smtp
config.action_mailer.perform_deliveries = true
@@ -69,9 +69,9 @@
config.action_mailer.default(charset: "utf-8")
config.action_mailer.smtp_settings = {
- address: ENV.fetch("MAILSERVER", nil),
+ address: ENV.fetch("MAILSERVER"),
port: 25,
- domain: ENV.fetch("MAILSERVER", nil)
+ domain: ENV.fetch("MAILSERVER")
}
# Ignore bad email addresses and do not raise email delivery errors.
diff --git a/config/initializers/default_setting.rb b/config/initializers/default_setting.rb
index bdc25a5da..49d0ea455 100644
--- a/config/initializers/default_setting.rb
+++ b/config/initializers/default_setting.rb
@@ -1,10 +1,10 @@
class DefaultSetting
- ERDBEERE_LINK = ENV.fetch("ERDBEERE_SERVER", nil)
- MUESLI_LINK = ENV.fetch("MUESLI_SERVER", nil)
- PROJECT_EMAIL = ENV.fetch("PROJECT_EMAIL", nil)
- PROJECT_NOTIFICATION_EMAIL = ENV.fetch("PROJECT_NOTIFICATION_EMAIL", nil)
- BLOG_LINK = ENV.fetch("BLOG", nil)
- URL_HOST_SHORT = ENV.fetch("URL_HOST_SHORT", nil)
+ ERDBEERE_LINK = ENV.fetch("ERDBEERE_SERVER")
+ MUESLI_LINK = ENV.fetch("MUESLI_SERVER")
+ PROJECT_EMAIL = ENV.fetch("PROJECT_EMAIL")
+ PROJECT_NOTIFICATION_EMAIL = ENV.fetch("PROJECT_NOTIFICATION_EMAIL")
+ BLOG_LINK = ENV.fetch("BLOG")
+ URL_HOST_SHORT = ENV.fetch("URL_HOST_SHORT")
RESEARCHGATE_LINK = "https://www.researchgate.net/project/MaMpf-Mathematische-Medienplattform".freeze
TOUR_LINK = "https://mampf.blog/ueber-mampf/".freeze
RESOURCES_LINK = "https://mampf.blog/ressourcen-fur-editorinnen/".freeze
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index c3b179a48..d900ea888 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -12,7 +12,7 @@
# note that it will be overwritten if you use your own mailer class
# with default "from" parameter.
config.mailer_sender = if Rails.env.production?
- ENV.fetch("FROM_ADDRESS", nil)
+ ENV.fetch("FROM_ADDRESS")
else
"please-change-me-at-config-initializers-devise@example.com"
end
diff --git a/config/initializers/thredded.rb b/config/initializers/thredded.rb
index 75f4539e3..30b0ae4a3 100644
--- a/config/initializers/thredded.rb
+++ b/config/initializers/thredded.rb
@@ -190,4 +190,4 @@
#
# add in (must install separate gem (under development) as well):
# Thredded.notifiers = [Thredded::EmailNotifier.new,
-# Thredded::PushoverNotifier.new(ENV.fetch("PUSHOVER_APP_ID", nil))]
+# Thredded::PushoverNotifier.new(ENV.fetch("PUSHOVER_APP_ID"))]
diff --git a/docker/production/Dockerfile b/docker/production/Dockerfile
index d7dd28f20..c1856e773 100644
--- a/docker/production/Dockerfile
+++ b/docker/production/Dockerfile
@@ -81,5 +81,36 @@ RUN bundle install
RUN yarn install --production=true
COPY --chown=app:app . /usr/src/app
+
+# The command ". ./docker-dummy.env" will source our dummy docker env file.
+# So why do we need this?
+#
+# Well, (deeply inhales), Rails needs to boot entirely to run the
+# `assets:precompile` task. Therefore, it also needs to access the env variables
+# to correctly start the initializers.
+#
+# However (after a long ime researching), docker compose does not seem to offer
+# an easy solution to have an env file from the host machine available during
+# the build step (Dockerfile) and not just during the run time of the container.
+# Note that the env file is indeed available on our host, just not in the build
+# context, the latter being the MaMpf github repo that docker compose pulls from.
+#
+# Even with volumes and bind mounts it's not working properly ("file not found").
+# In the end, we found a solution that suggests to use the new docker buildkit
+# to allow for multiple build contexts. Yet, we explicitly set DOCKER_BUILDKIT=0
+# to use the old buildkit since the new one always gives a moby-related ssh error.
+# And even if this worked, it is not entirely clear if this is even working
+# with docker compose or just with docker (sigh).
+#
+# That's why, in the end, we decided to leverage our already-existing dummy env
+# file and source it here in the Dockerfile just to have the precompile task run
+# successfully (this task doesn't even rely on the actual values, so despite
+# being a hack, it should be fine).
+#
+# I've written down more details in this question on StackOverflow:
+# https://stackoverflow.com/q/78098380/
+COPY ./docker/production/docker.env ./docker-dummy.env
+
RUN cp -r $(bundle info --path sidekiq)/web/assets /usr/src/app/public/sidekiq && \
- SECRET_KEY_BASE="$(bundle exec rails secret)" DB_ADAPTER=nulldb bundle exec rails assets:precompile
+ set -o allexport && . ./docker-dummy.env && set +o allexport && \
+ SECRET_KEY_BASE="$(bundle exec rails secret)" DB_ADAPTER=nulldb bundle exec rails assets:precompile
diff --git a/docker/production/docker.env b/docker/production/docker.env
index 8070eb0d9..9704dd428 100644
--- a/docker/production/docker.env
+++ b/docker/production/docker.env
@@ -1,4 +1,5 @@
-GIT_BRANCH=production
+# Instance variables
+GIT_BRANCH=main
COMPOSE_PROJECT_NAME=mampf
INSTANCE_NAME=mampf
@@ -14,15 +15,23 @@ MEMCACHED_SERVER=cache
FROM_ADDRESS=mampf@mathi.uni-heidelberg.de
MAILSERVER=mail.mathi.uni-heidelberg.de
PROJECT_EMAIL=mampf@mathi.uni-heidelberg.de
-ERROR_EMAIL=mampf-error@mathi.uni-heidelberg.de
+PROJECT_NOTIFICATION_EMAIL=notificationmail
MAILID_DOMAIN=mathi.uni-heidelberg.de
+ERROR_EMAIL=mampf-error@mathi.uni-heidelberg.de
IMAPSERVER=mail.mathi.uni-heidelberg.de
PROJECT_EMAIL_USERNAME=creativeusername
PROJECT_EMAIL_PASSWORD=secretsecret
-PROJECT_EMAIL_MAILBOX=Other Users/mampf
+PROJECT_EMAIL_MAILBOX="Other Users/mampf"
# Due to CORS constraints, some urls are proxied to the media server
DOWNLOAD_LOCATION=https://mampf.mathi.uni-heidelberg.de/mediaforward
+REWRITE_ENABLED=1
+
+# Captcha Server
+USE_CAPTCHA_SERVICE=1
+CAPTCHA_VERIFY_URL=https://captcha2go.mathi.uni-heidelberg.de/v1/verify
+CAPTCHA_PUZZLE_URL=https://captcha2go.mathi.uni-heidelberg.de/v1/puzzle
+CAPTCHA_APPLICATION_TOKEN=secret
# Upload folder
MEDIA_PATH=/private/media
@@ -35,14 +44,14 @@ PRODUCTION_DATABASE_INTERACTIONS=mampf_interactions
PRODUCTION_DATABASE_HOST=db
PRODUCTION_DATABASE_USERNAME=mampf
PRODUCTION_DATABASE_PASSWORD=supersecret
-PRODUCTION_DATABASE_PORT=5432
-PRODUCTION_DATABASE_URL='postgresql://mampf:supersecret@db:5432/mampf'
+PRODUCTION_DATABASE_PORT=port
+PRODUCTION_DATABASE_URL='postgresql://mampf:supersecret@db:port/mampf'
# Rails configuration
# change RAILS_ENV to production for a production deployment
RAILS_ENV=production
-RAILS_MASTER_KEY=
-SECRET_KEY_BASE=
+RAILS_MASTER_KEY=secret
+SECRET_KEY_BASE=secret
URL_HOST=mampf.mathi.uni-heidelberg.de
URL_HOST_SHORT=http://mampf.media
diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake
index d7aa4dd01..067cd8ac2 100644
--- a/lib/tasks/db.rake
+++ b/lib/tasks/db.rake
@@ -33,7 +33,7 @@
namespace :db do
desc "Dumps the database to backups"
task dump: :environment do
- dump_fmt = ensure_format(ENV.fetch("format", nil))
+ dump_fmt = ensure_format(ENV.fetch("format"))
dump_sfx = suffix_for_format(dump_fmt)
backup_dir = backup_directory(Rails.env, create: true)
full_path = nil
@@ -56,10 +56,10 @@ namespace :db do
namespace :dump do
desc "Dumps a specific table to backups"
task table: :environment do
- table_name = ENV.fetch("table", nil)
+ table_name = ENV.fetch("table")
if table_name.present?
- dump_fmt = ensure_format(ENV.fetch("format", nil))
+ dump_fmt = ensure_format(ENV.fetch("format"))
dump_sfx = suffix_for_format(dump_fmt)
backup_dir = backup_directory(Rails.env, create: true)
full_path = nil
@@ -92,7 +92,7 @@ namespace :db do
desc "Restores the database from a backup using PATTERN"
task restore: :environment do
- pattern = ENV.fetch("pattern", nil)
+ pattern = ENV.fetch("pattern")
if pattern.present?
file = nil
From 469bb11377568b68e11d4b850457dc59f5d3f4b0 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Tue, 19 Mar 2024 12:10:20 +0100
Subject: [PATCH 13/31] Authenticate mails sent by MaMpf (#590)
* Authenticate mails sent by MaMpf
* Fix mails comment
* Add `nil` env fallback due to #542 (this one was removed again in merge commit 4d066c3)
---
config/environments/production.rb | 3 ++-
docker/production/docker.env | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/config/environments/production.rb b/config/environments/production.rb
index ec2a4e303..ae9c17db1 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -71,7 +71,8 @@
config.action_mailer.smtp_settings = {
address: ENV.fetch("MAILSERVER"),
port: 25,
- domain: ENV.fetch("MAILSERVER")
+ user_name: ENV.fetch("MAMPF_EMAIL_USERNAME"),
+ password: ENV.fetch("MAMPF_EMAIL_PASSWORD")
}
# Ignore bad email addresses and do not raise email delivery errors.
diff --git a/docker/production/docker.env b/docker/production/docker.env
index 9704dd428..11523738e 100644
--- a/docker/production/docker.env
+++ b/docker/production/docker.env
@@ -11,7 +11,7 @@ MUESLI_SERVER=https://muesli.mathi.uni-heidelberg.de
ERDBEERE_API=https://erdbeere.mathi.uni-heidelberg.de/api/v1
MEMCACHED_SERVER=cache
-# Email is send using a mailserver without authentication. Specify how to connect here
+# Email
FROM_ADDRESS=mampf@mathi.uni-heidelberg.de
MAILSERVER=mail.mathi.uni-heidelberg.de
PROJECT_EMAIL=mampf@mathi.uni-heidelberg.de
@@ -22,6 +22,8 @@ IMAPSERVER=mail.mathi.uni-heidelberg.de
PROJECT_EMAIL_USERNAME=creativeusername
PROJECT_EMAIL_PASSWORD=secretsecret
PROJECT_EMAIL_MAILBOX="Other Users/mampf"
+MAMPF_EMAIL_USERNAME=secret
+MAMPF_EMAIL_PASSWORD=secret
# Due to CORS constraints, some urls are proxied to the media server
DOWNLOAD_LOCATION=https://mampf.mathi.uni-heidelberg.de/mediaforward
From 4e031a6adefa80a48df00e2f4d315173138a5c79 Mon Sep 17 00:00:00 2001
From: Splines <37160523+Splines@users.noreply.github.com>
Date: Wed, 20 Mar 2024 23:25:53 +0100
Subject: [PATCH 14/31] Add feedback button (#527)
Due to a forced push of the dev branch, the following list might
unfortunately include items from other branches as well.
* Init Feedback model
* Add Feedback modal view and corresponding controller
First working version, of course still a lot to improve from here.
* Migrate feedback form to Bootstrap v5
* Add basic styling to Feedback form
* Add "allow contact via mail" checkbox
A new column was added to the Feedbacks schema.
Note that we did not create a new migration as this is a PR which should
only contain one migration, namely the one for the creation of the whol
Feedback table.
* Toggle "allow email contact" by default
* Improve submit button handler (outsource to function)
* Init feedback mailer
Right now just for ourselves, so that we get a plaintext mail with
the feedback of a user.
Env variables were adjusted accordingly, but need to be set manually
in the production environment!
* Adjust feedback mail in views
* Implement success/error flow with toast messages
* Add missing database field "can_contact"
* Add internationalization to feedback error/success
* Lint some files
* Set feedback text field as required with min 10 chars
* Add "optional" to title in email
* Adjust spacing around feedback button
* Internationalize tooltip
* Delete console log
* Add comment describing hidden submit button handler
* Delete default test specs
* Add proper validation for Feedback body
Alongside this, also made sure that we use a custom client-side
validation message when input is too short (under 10 chars long).
This allows us to use the language the user has selected in MaMpf
instead of the browser language.
* Default `can_contact` to false in backend
* Update bootstrap to v5.3.1
command used: bundle update bootstrap
bundle update bootstrap --conservative did not work, as docker
containers did not start again due to dependency errors
* Revert "Update bootstrap to v5.3.1" in favor of PR #537
This reverts commit 5cd1af25820f0eea73a98a99073634ef77daa75a.
* Submit form via Ctrl + Enter when modal is opened
* Remove default nil value from ENV.fetch()
* Revert "Remove default nil value from ENV.fetch()"
This reverts commit 696a395cce1bb24d1018e84479a85cc0a66d77ba.
* Rename button to 'Send' (not 'Save')
* Check if should register feedback event handlers
* Make feedback button ID more specific
* Fix line wrapping (code style)
* Use delete on cascade to be able to delete a user
even if he/she has sent some feedback
* Move Send button before Cancel button
* Replace "on delete cascade" with "dependent destroy"
* Add cypress rules to ESLint & ignore some patterns
* Allow usage of tempusDominus global variable
* Ignore JS files with Sprocket syntax
* Further improve rules, e.g. allow common globals
* Ignore sprocket syntax in cable.js
* Autofix all `.js` and `.js.erb` files
Command used:
`yarn run eslint --fix .`
Still 47 problems (27 errors, 20 warnings) after this.
* Fix variables in turbolink fix
* Prepend unused variables with "_"
* Get rid of unused widget variable
* Fix specs comment tab alignment
* Warn about too long GitHub commit messages (#586)
* Fix comment status (#585)
* Reapply first fix for Reader/Media
See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.
* Correctly show latest post (might be current_user's comment)
* Fix update of unread comments logic in comments controller
* Fix update icon logic and latest post comment
* Simplify latest comment logic
* Improve code comments
* Further improve comments
* Fix wording in comment
* Fix construction of media array & use `.blank?` instead of `.empty?`
* Migrate `unread_comments` flag (fix inconsistencies) (#587)
* Add dummy migration
* Implement migration for unread comment flag
* Remove unnecessary comment
* Declare migration as not idempotent
* Use array.length instead of counting
* Throw error to prevent revert of migration
* Fix severe flaws in unread comments migration
* Simplify Reader retrieval
* Use the more explicit `.nil?` method
* Update migration date
* Fix annoying bug: don't use `.select!` but `.select`
* Polish migration
e.g. update comment, more suitable name for the method etc.
* Rename method according to #585
* Use `warn` log level for migration (#588)
* Fix linting in feedback.js
* Fix RuboCop errors
* Fix remaining ESLint errors
* Update timestamp of feedback migration
* Add missing Feedback email to prod docker.env
* Remove unnecessary Feedback env variables
* Add validation message for empty body
* Change `const` to `var` to avoid "redefined" errors
* Update timestamp of feedback migration (again)
---
app/assets/javascripts/feedback.js | 74 +++++++++++++++++++
app/assets/javascripts/lectures.coffee | 2 +
app/assets/stylesheets/application.scss | 4 +
app/assets/stylesheets/feedback.scss | 11 +++
app/controllers/feedbacks_controller.rb | 21 ++++++
app/mailers/feedback_mailer.rb | 15 ++++
app/models/feedback.rb | 10 +++
app/models/user.rb | 3 +
.../new_user_feedback_email.text.erb | 13 ++++
app/views/feedbacks/_feedback.html.erb | 44 +++++++++++
app/views/feedbacks/_feedback_button.html.erb | 8 ++
app/views/feedbacks/_feedback_form.html.erb | 63 ++++++++++++++++
app/views/feedbacks/create.js.erb | 7 ++
.../shared/_dropdown_notifications.html.erb | 4 +-
app/views/shared/_footer.html.erb | 2 +-
app/views/shared/_navbar.html.erb | 7 ++
config/initializers/default_setting.rb | 1 +
config/locales/de.yml | 30 ++++++++
config/locales/en.yml | 29 ++++++++
config/routes.rb | 3 +
db/migrate/20240319130000_create_feedbacks.rb | 12 +++
db/schema.rb | 13 +++-
docker/development/docker-compose.yml | 1 +
docker/production/docker.env | 1 +
.../docker-compose.local.yml | 1 +
docker/run_cypress_tests/docker-compose.yml | 1 +
26 files changed, 376 insertions(+), 4 deletions(-)
create mode 100644 app/assets/javascripts/feedback.js
create mode 100644 app/assets/stylesheets/feedback.scss
create mode 100644 app/controllers/feedbacks_controller.rb
create mode 100644 app/mailers/feedback_mailer.rb
create mode 100644 app/models/feedback.rb
create mode 100644 app/views/feedback_mailer/new_user_feedback_email.text.erb
create mode 100644 app/views/feedbacks/_feedback.html.erb
create mode 100644 app/views/feedbacks/_feedback_button.html.erb
create mode 100644 app/views/feedbacks/_feedback_form.html.erb
create mode 100644 app/views/feedbacks/create.js.erb
create mode 100644 db/migrate/20240319130000_create_feedbacks.rb
diff --git a/app/assets/javascripts/feedback.js b/app/assets/javascripts/feedback.js
new file mode 100644
index 000000000..7d9ab3772
--- /dev/null
+++ b/app/assets/javascripts/feedback.js
@@ -0,0 +1,74 @@
+$(document).on("turbolinks:load", () => {
+ if (!shouldRegisterFeedback()) {
+ return;
+ }
+ registerToasts();
+ registerSubmitButtonHandler();
+ registerFeedbackBodyValidator();
+});
+
+var SUBMIT_FEEDBACK_ID = "#submit-feedback";
+
+var TOAST_OPTIONS = {
+ animation: true,
+ autohide: true,
+ delay: 6000, // autohide after ... milliseconds
+};
+
+function shouldRegisterFeedback() {
+ return $(SUBMIT_FEEDBACK_ID).length > 0;
+}
+
+function registerToasts() {
+ const toastElements = document.querySelectorAll(".toast");
+ [...toastElements].map((toast) => {
+ new bootstrap.Toast(toast, TOAST_OPTIONS);
+ });
+}
+
+function registerSubmitButtonHandler() {
+ // Invoke the hidden submit button inside the actual Rails form
+ $("#submit-feedback-form-btn-outside").click(() => {
+ submitFeedback();
+ });
+
+ // Submit form by pressing Ctrl + Enter
+ document.addEventListener("keydown", (event) => {
+ const isModalOpen = $(SUBMIT_FEEDBACK_ID).is(":visible");
+ if (isModalOpen && event.ctrlKey && event.key == "Enter") {
+ submitFeedback();
+ }
+ });
+}
+
+function registerFeedbackBodyValidator() {
+ const feedbackBody = document.getElementById("feedback_feedback");
+ feedbackBody.addEventListener("input", () => {
+ validateFeedback();
+ });
+}
+
+function validateFeedback() {
+ const feedbackBody = document.getElementById("feedback_feedback");
+ const validityState = feedbackBody.validity;
+ if (validityState.tooShort) {
+ const tooShortMessage = feedbackBody.dataset.tooShortMessage;
+ feedbackBody.setCustomValidity(tooShortMessage);
+ }
+ else if (validityState.valueMissing) {
+ const valueMissingMessage = feedbackBody.dataset.valueMissingMessage;
+ feedbackBody.setCustomValidity(valueMissingMessage);
+ }
+ else {
+ // render input valid, so that form will submit
+ feedbackBody.setCustomValidity("");
+ }
+
+ feedbackBody.reportValidity();
+}
+
+function submitFeedback() {
+ const submitButton = $("#submit-feedback-form-btn");
+ validateFeedback();
+ submitButton.click();
+}
diff --git a/app/assets/javascripts/lectures.coffee b/app/assets/javascripts/lectures.coffee
index 15dfacca0..5e47d4fea 100644
--- a/app/assets/javascripts/lectures.coffee
+++ b/app/assets/javascripts/lectures.coffee
@@ -214,6 +214,7 @@ $(document).on 'turbolinks:load', ->
$('#secondnav').show()
$('#lecturesDropdown').appendTo($('#secondnav'))
$('#notificationDropdown').appendTo($('#secondnav'))
+ $('#feedback-btn').appendTo($('#secondnav'))
$('#searchField').appendTo($('#secondnav'))
$('#second-admin-nav').show()
$('#adminDetails').appendTo($('#second-admin-nav'))
@@ -236,6 +237,7 @@ $(document).on 'turbolinks:load', ->
$('#secondnav').hide()
$('#lecturesDropdown').appendTo($('#firstnav'))
$('#notificationDropdown').appendTo($('#firstnav'))
+ $('#feedback-btn').appendTo($('#firstnav'))
$('#searchField').appendTo($('#firstnav'))
$('#second-admin-nav').hide()
$('#teachableDrop').appendTo($('#first-admin-nav'))
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
index 35f8c0d78..4fed1ddea 100644
--- a/app/assets/stylesheets/application.scss
+++ b/app/assets/stylesheets/application.scss
@@ -302,4 +302,8 @@ a {
&:hover {
text-decoration: underline;
}
+}
+
+.toast {
+ background-color: white;
}
\ No newline at end of file
diff --git a/app/assets/stylesheets/feedback.scss b/app/assets/stylesheets/feedback.scss
new file mode 100644
index 000000000..7f14ad18c
--- /dev/null
+++ b/app/assets/stylesheets/feedback.scss
@@ -0,0 +1,11 @@
+#feedback-btn {
+ color: white;
+
+ &:focus {
+ box-shadow: none;
+ }
+
+ &:hover {
+ color: #ffc107;
+ }
+}
\ No newline at end of file
diff --git a/app/controllers/feedbacks_controller.rb b/app/controllers/feedbacks_controller.rb
new file mode 100644
index 000000000..9ae31a957
--- /dev/null
+++ b/app/controllers/feedbacks_controller.rb
@@ -0,0 +1,21 @@
+class FeedbacksController < ApplicationController
+ authorize_resource except: [:create]
+
+ def create
+ feedback = Feedback.new(feedback_params)
+ feedback.user_id = current_user.id
+ @feedback_success = feedback.save
+
+ if @feedback_success
+ FeedbackMailer.with(feedback: feedback).new_user_feedback_email.deliver_later
+ end
+
+ respond_to(&:js)
+ end
+
+ private
+
+ def feedback_params
+ params.require(:feedback).permit(:title, :feedback, :can_contact)
+ end
+end
diff --git a/app/mailers/feedback_mailer.rb b/app/mailers/feedback_mailer.rb
new file mode 100644
index 000000000..152b1378b
--- /dev/null
+++ b/app/mailers/feedback_mailer.rb
@@ -0,0 +1,15 @@
+class FeedbackMailer < ApplicationMailer
+ default from: DefaultSetting::FEEDBACK_EMAIL
+ layout false
+
+ # Mail to the MaMpf developers including the new feedback of a user.
+ def new_user_feedback_email
+ @feedback = params[:feedback]
+ reply_to_mail = @feedback.can_contact ? @feedback.user.email : ""
+ subject = "Feedback: #{@feedback.title}"
+ mail(to: DefaultSetting::FEEDBACK_EMAIL,
+ subject: subject,
+ content_type: "text/plain",
+ reply_to: reply_to_mail)
+ end
+end
diff --git a/app/models/feedback.rb b/app/models/feedback.rb
new file mode 100644
index 000000000..2cc0a0c6b
--- /dev/null
+++ b/app/models/feedback.rb
@@ -0,0 +1,10 @@
+# Feedback from users regarding MaMpf itself.
+class Feedback < ApplicationRecord
+ belongs_to :user
+
+ BODY_MIN_LENGTH = 10
+ BODY_MAX_LENGTH = 10_000
+ validates :feedback, length: { minimum: BODY_MIN_LENGTH,
+ maximum: BODY_MAX_LENGTH },
+ allow_blank: false
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 40feb6f60..4b612f20e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -78,6 +78,9 @@ class User < ApplicationRecord
# a user has a watchlist with watchlist_entries
has_many :watchlists, dependent: :destroy
+
+ has_many :feedbacks, dependent: :destroy
+
include ScreenshotUploader[:image]
# if a homepage is given it should at leat be a valid address
diff --git a/app/views/feedback_mailer/new_user_feedback_email.text.erb b/app/views/feedback_mailer/new_user_feedback_email.text.erb
new file mode 100644
index 000000000..9b580a4a4
--- /dev/null
+++ b/app/views/feedback_mailer/new_user_feedback_email.text.erb
@@ -0,0 +1,13 @@
+# Title (optional)
+<%= @feedback.title %>
+
+# Feedback
+<%= @feedback.feedback %>
+
+
+-----
+<% if @feedback.can_contact %>
+Reply to this mail to contact the user.
+<% else %>
+User did not give permission to contact them regarding their feedback, so we cannot reply to this mail.
+<% end %>
diff --git a/app/views/feedbacks/_feedback.html.erb b/app/views/feedbacks/_feedback.html.erb
new file mode 100644
index 000000000..b98f5c8bc
--- /dev/null
+++ b/app/views/feedbacks/_feedback.html.erb
@@ -0,0 +1,44 @@
+<%= stylesheet_link_tag 'feedback' %>
+
+<%# Main Modal %>
+