I’m testing a feature in my current dev branch and would like some feedback. The technical side of it is complete, but I’m curious about the human side of it.

When loading a removed comment in the comment section, it automatically fetches the removal reason from the modlog and appends it below 'Removed by Mod" on the comment. The “Removed by Mod” text is also linked to the modlog entry for the item.

Additionally, if you’re a mod, it will also append, in a spoiler block, the original comment that was removed. Somewhere between 0.19.3 and 0.19.9, the Lemmy devs decided mods should no longer be able to see removed comments in their own communities, which I think is a huge regression (though thankfully admins can still see them).

Thoughts? Is this asking for drama, or would it be generally beneficial? Right now, in the dev branch, it just does it, but I can make it a user option.

  • Ledivin@lemmy.world
    4·
    14 days ago

    I like the idea, but are there any performance concerns when a post is a graveyard with lots of removed comments?

    • Admiral Patrick@lemmy.world
      2·
      14 days ago

      I did test a few posts with a lot of removed comments (both on my instance and Lemmy World), and the overhead wasn’t terrible since scoping the modlog to just the comment ID is pretty lightweight. Since HTTP/2 is pretty common (which can re-use connections), there’s not overhead of additional TLS handshakes slowing things down.

      From a network traffic standpoint, Gzip compressed JSON is pretty negligible in the grand scheme of things.

      • moseschrute@lemmy.world
        2·
        14 days ago

        What about Lemmy’s rate limiting?

        I’m working on a client, and I would be worried that making too many requests for a nice to have feature would rate limit a request for a core feature of the app. Though I supposed some sort of throttle queue would solve that.

        I’m already using a throttle queue to slow down refreshing stale data in my app. That way data that I’m asking for right now takes priority over refreshing a bunch of old data.

        • Admiral Patrick@lemmy.world
          2·
          14 days ago

          That’s a good point. I’ll have to check the default values, but on my own instance, I have very conservative limits in place, and it hasn’t proven to be an issue (so far?).

          Unless it’s changed since I wrote the online docs for Tessreact, the modlog is part of the “Messages” rate limit bucket which is/was something of a catch-all for endpoints that didn’t fit elsewhere. Even in the default config, that bucket is the most permissive due to that.

          I’ve been daily-driving my dev version with this feature enabled for a few days, and it hasn’t been an issue so far (it only does a modlog lookup if a comment is removed, so not on every comment in the tree). It’s also per IP, so unless a lot of people are behind the same public IP, I don’t think it’s going to pose an issue. I’d have to double check, but I think the most comments it loads in a batch is close to 100, so unless every comment has been removed, that would be the worst-case number of modlog fetches.

          So it looks like I’ll definitely want to make this feature toggleable even if it does end up defaulting to ‘on’.

    • Admiral Patrick@dubvee.orgOPEnglish
      2·
      14 days ago

      That was the original plan, but I’m trying to achieve four goals with a single API call:

      1. (Optionally?) Show the removal reason for everyone
      2. For mods only, provide the original comment text for reference since the Lemmy devs took that away despite it being useful. The original text is pulled from the same modlog lookup that also fetches the reason.
      3. For admins, continue showing the current comment value (in lieu of “Removed by Mod”) since the content is available to them.
      4. Not to over-complicate a feature I’m going to have to re-write anyway when 1.0 comes out and breaks most of the API. 😠

      That’s why I was thinking that making a user option for the behavior would be the compromise (the same setting toggle would control both 1 and 2 while achieving #4).

      • can@sh.itjust.works
        2·
        14 days ago

        That’s why I was thinking that making a user option for the behavior would be the compromise (the same setting toggle would control both 1 and 2 while achieving #4).

        Looks reasonable to me. Especially given the new API changes. How rough is the prepwork for that looking to be?

        • Admiral Patrick@dubvee.orgOPEnglish
          2·
          14 days ago

          How rough is the prepwork for that looking to be?

          So far, medium to major PITA. AFAIK, there’s still not a full list of things that they’re breaking in v3 (haven’t even looked at going full v4 yet, but assuming major PITA). So I’m still waiting for the other shoe to drop before diving in. I don’t have time to chase a moving target any more than I already am.

          • can@sh.itjust.works
            2·
            14 days ago

            I see, in other words I should start getting used to trying apps other than Sync. At least the Lemmy devs seem to appreciate this is a big task and intend to giving ample time.

            • Admiral Patrick@dubvee.orgOPEnglish
              1·
              14 days ago

              Yeah, we’ll see. I’d like to have a concrete list of changes/breaks before starting to port things to Lemmy yet again. If only they’d put all their breaking changes in v4, it would be nice.