DEV: Remove defunct TODOs (#19825)

* Firefox now finally returns PerformanceMeasure from performance.measure
* Some TODOs were really more NOTE or FIXME material or no longer relevant
* retain_hours is not needed in ExternalUploadsManager,  it doesn't seem like anywhere in the UI sends this as a param for uploads
* https://github.com/discourse/discourse/pull/18413 was merged so we can remove JS test workaround for settings

app/assets/javascripts/discourse/app/components/composer-editor.js

@@ -106,8 +106,6 @@ export default Component.extend(ComposerUploadUppy, {
   fileUploadElementId: "file-uploader",
   mobileFileUploaderId: "mobile-file-upload",
 
-  // TODO (martin) Remove this once the chat plugin is using the new composerEventPrefix
-  eventPrefix: "composer",
   composerEventPrefix: "composer",
   uploadType: "composer",
   uppyId: "composer-editor-uppy",

app/assets/javascripts/discourse/app/mixins/extendable-uploader.js

@@ -74,7 +74,7 @@ export default Mixin.create(UploadDebugging, {
     }
   },
 
-  // TODO (martin) This and _onPreProcessComplete will need to be tweaked
+  // NOTE: This and _onPreProcessComplete will need to be tweaked
   // if we ever add support for "determinate" preprocessors for uppy, which
   // means the progress will have a value rather than a started/complete
   // state ("indeterminate").

app/assets/javascripts/discourse/app/mixins/upload-debugging.js

@@ -31,7 +31,6 @@ export default Mixin.create({
 
   _instrumentUploadTimings() {
     if (!this._performanceApiSupport()) {
-      // TODO (martin) (2021-01-23) Check if FireFox fixed this yet.
       warn(
         "Some browsers do not return a PerformanceMeasure when calling performance.mark, disabling instrumentation. See https://developer.mozilla.org/en-US/docs/Web/API/Performance/measure#return_value and https://bugzilla.mozilla.org/show_bug.cgi?id=1724645",
         { id: "discourse.upload-debugging" }

app/models/external_upload_stub.rb

@@ -43,9 +43,6 @@ class ExternalUploadStub < ActiveRecord::Base
     @statuses ||= Enum.new(created: 1, uploaded: 2, failed: 3)
   end
 
-  # TODO (martin): Lifecycle rule would be best to clean stuff up in the external
-  # systems, I don't think we really want to be calling out to the external systems
-  # here right?
   def self.cleanup!
     expired_created.delete_all
     expired_uploaded.delete_all

app/services/external_upload_manager.rb

@@ -149,8 +149,6 @@ class ExternalUploadManager
       raise ChecksumMismatchError if external_sha1 && external_sha1 != actual_sha1
     end
 
-    # TODO (martin): See if these additional opts will be needed
-    # - check if retain_hours is needed
     opts = {
       type: external_upload_stub.upload_type,
       existing_external_upload_key: external_upload_stub.key,

lib/s3_helper.rb

@@ -175,12 +175,9 @@ class S3Helper
           cors_rules: final_rules,
         },
       )
-    rescue Aws::S3::Errors::AccessDenied => err
-      # TODO (martin) Remove this warning log level once we are sure this new
-      # ensure_cors! rule is functioning correctly.
-      Discourse.warn_exception(
-        err,
-        message: "Could not PutBucketCors rules for #{@s3_bucket_name}, rules: #{final_rules}",
+    rescue Aws::S3::Errors::AccessDenied
+      Rails.logger.info(
+        "Could not PutBucketCors rules for #{@s3_bucket_name}, rules: #{final_rules}",
       )
       return false
     end

lib/upload_creator.rb

@@ -57,9 +57,6 @@ class UploadCreator
         true
       end
     )
-
-    # TODO (martin) Validate @opts[:type] to make sure only blessed types are passed
-    # in, since the clientside can pass any type it wants.
   end
 
   def create_for(user_id)
@@ -78,13 +75,13 @@ class UploadCreator
     is_image ||= @image_info && FileHelper.is_supported_image?("test.#{@image_info.type}")
     is_image = false if @opts[:for_theme]
 
-    # if this is present then it means we are creating an upload record from
+    # If this is present then it means we are creating an upload record from
     # an external_upload_stub and the file is > ExternalUploadManager::DOWNLOAD_LIMIT,
     # so we have not downloaded it to a tempfile. no modifications can be made to the
     # file in this case because it does not exist; we simply move it to its new location
     # in S3
     #
-    # TODO (martin) I've added a bunch of external_upload_too_big checks littered
+    # FIXME: I've added a bunch of external_upload_too_big checks littered
     # throughout the UploadCreator code. It would be better to have two seperate
     # classes with shared methods, rather than doing all these checks all over the
     # place. Needs a refactor.

plugins/chat/test/javascripts/unit/lib/chat-emoji-reaction-store-test.js

@@ -16,13 +16,10 @@ module("Discourse Chat | Unit | chat-emoji-reaction-store", function (hooks) {
     this.chatEmojiReactionStore.reset();
   });
 
-  // TODO (martin) Remove site setting workarounds after core PR#1290
   test("defaults", function (assert) {
     assert.deepEqual(
       this.chatEmojiReactionStore.favorites,
-      (this.siteSettings.default_emoji_reactions || "")
-        .split("|")
-        .filter((val) => val)
+      this.siteSettings.default_emoji_reactions.split("|").filter((val) => val)
     );
   });