[WIP] fix bug preventing tags of mismatched case from being removed

[sunny-b]

What type of PR is this? (check all applicable)

• Bug Fix

Description

Tag mods are currently not able to remove tags from articles is the tags have mismatched cases, even if they are the same tag (ex. git vs. Git)

This PR makes it so mods are able remove tags regardless of case sensitivity by downcasing tag names when a Tag adjustment is created. However, I put this PR in WIP at the moment because my fix is mainly addressing the symptom while there is another possible solution that would address the root of the issue.

If tag names were transformed to lowercase when an article is created, it would prevent any edge cases like this arising. It would be a simple matter of calling .map(&:downcase) at these two lines.

https://github.com/sunny-b/dev.to/blob/9bd875670e0d7b15bacf67f55eb6cb915d866e9e/app/services/articles/creator.rb#L44

https://github.com/sunny-b/dev.to/blob/9bd875670e0d7b15bacf67f55eb6cb915d866e9e/app/services/articles/updater.rb#L28

However, that would have the side-effect of making all tags appear in lowercase. I'm not sure if that's a no-go or not so figured I'd bring it up to the other contributors.

Related Tickets & Documents

#5325

Recordings

screen recording

Added to documentation?

• no documentation needed

[optional] What gif best describes this PR or how it makes you feel?

cc: @rhymes @mstruve @citizen428

[citizen428]

Thanks for this @sunny-b 🎉

I do however agree that this only patches over the symptom instead of fixing the root cause. Note that tags (the Tag model), already can have a name and a pretty_name. As tag moderator I e.g. use that to have the tag "fsharp" appear as "F#" at the top of the tag page. We can potentially use this (or something similar) for a more robust solution than simply downcasing everything on creation. Maybe @benhalpern or @maestromac want to weigh in on this.

[sunny-b]

@benhalpern @maestromac, do either of you have opinions on this? This PR, as is, will allow tag mods to remove tags regardless of string case. However, @citizen428 and I were discussing possible longer-term solutions to prevent this type of edge case.

[citizen428]

Sorry for the delay @sunny-b, someone will hopefully get back to you shortly.

[maestromac]

Thanks for this @sunny-b . I like your fix and I think that they are still useful even though they only fix the symptoms. I can see a scenario that improper tags could linger around in the future.

Regarding your suggestion, It is the right idea but that isn't going to work because it's tag_list we're dealing with (I need to look into why it's checking for tag there 🤔 ). We are already doing the same on the front matter articles (v1 editor) via TagParser on within Article Model. After some digging around, I think the quickest fix we can apply is to add the following to the beginning of Article#evalute_markdown

    if tag_list
      temp_tag_list = tag_list
      self.tag_list = [] # erase the tag
      tag_list.add(temp_tag_list, parser: ActsAsTaggableOn::TagParser)
    end

This keeps the tag parsing within Article model and keeps toe-stepping minimal between the two ways we currently parse article ( with and without front_matter). Let me know what you think.

[sunny-b]

@maestromac That's a good idea. I will include that and make a spec for it sometime this week or next when I get a little time and will ping you.

[rhymes]

Hello @sunny-b, as some time has passed, we were wondering if it would be okay for you if we completed this PR in your stead. You will still appear as a co-author in the commit log.

Let us know if you can't work on it.

Thank you for kickstarting the whole thing!

[mstruve]

Hi @sunny-b!

Thank you so much for the PR and for contributing to DEV! We are super happy to have your help 😄

With that said, we are in the process of going through and cleaning up some of our PR history to help us better prioritize what we need to be focusing on. Since this PR has not been active for a while I am going to go ahead and close it.

If you would still like to get this merged yourself please respond and let us know. In the meantime, if we dont hear from you in the next couple of days we will plan on bringing this in house to finish up.

Thanks again for contributing! Ping us anytime if you have questions!