38
Root
5y

Root gets ignored.

I've been working on this monster ticket for a week and a half now (five days plus other tickets). It involves removing all foreign keys from mass assignment (create, update, save, ...), which breaks 1780 specs.

For those of you who don't know, this is part of how rails works. If you create a Page object, you specify the book_id of its parent Book so they're linked. (If you don't, they're orphans.) Example: `Page.create(text: params[:text], book_id: params[:book_id], ...)` or more simply: `Page.create(params)`

Obviously removing the ability to do this is problematic. The "solution" is to create the object without the book_id, save it, then set the book_id and save it again. Two roundtrips. bad.

I came up with a solution early last week that, while it doesn't resolve the security warnings, it does fix the actual security issue: whitelisting what params users are allowed to send, and validating them. (StrongParams + validation). I had a 1:1 with my boss today about this ticket, and I told him about that solution. He sort of hand-waved it away and said it wouldn't work because <lots of unrelated things>. huh.

He worked through a failed spec to see what the ticket was about, and eventually (20 minutes later) ran into the same issues Idid, and said "there's no way around this" (meaning what security wants won't actually help).

I remembered that Ruby has a `taint` state tracking, and realized I could use that to write a super elegant drop-in solution: some Rack middleware or a StrongParams monkeypatch to mark all foreign keys from user-input as tainted (so devs can validate and un-taint them), and also monkeypatch ACtiveRecord's create/save/update/etc. to raise an exception when seeing tainted data. I brought this up, and he searched for it. we discovered someone had already build this (not surprising), but also that Ruby2.7 deprecates the `taint` mechanism literally "because nobody uses it." joy. Boss also somehow thought I came up with it because I saw the other person's implementation, despite us searching for it because I brought it up? 🤨

Foregoing that, we looked up more possibilities, and he saw the whitelist+validation pattern quite a few more times, which he quickly dimissed as bad, and eventually decided that we "need to noodle on it for awhile" and come up with something else.

Shortly (seriously 3-5 minutes) after the call, he said that the StrongParams (whitelist) plus validation makes the most sense and is the approach we should use.

ffs.
I came up with that last week and he said no.
I brought it up multiple times during our call and he said it was bad or simply talked over me. He saw lots of examples in the wild and said it was bad. I came up with a better, more elegant solution, and he credited someone else. then he decided after the call that the StrongParams idea he came up with (?!) was better.

jfc i'm getting pissy again.

Comments
  • 3
    See, this is why I'm somewhat scared of working in that branch in the future.
    God the guy sucks
  • 0
    that sounds like a person, which knows about leadership. not.
  • 1
    If you call him on this he will just mansplain it. I wonder if he has an internal dialog that doesn't include other people.

    Also, if for some reason it doesn't work you can blame him.
  • 8
    Oh, and I finally managed to get him to admit that the ticket's scope is ridiculously large.

    In his words: "completely overwhelming and completely unrealistic," amounts to "several months of work," and should be broken up.

    Bloody finally.
    How did he not realize this before? did he not pay attention? did he not care?

    As much as Ilike working on something until it's finished, I kinda want to see progress and cross things off. and I totally don't want to double or triple (or 10x) my workload with the endless merge conflicts that would absolutely happen since I'm rewriting part of basically every model and controller in the application while there's active development going on.

    jfc again.
  • 4
    @Demolishun Mansplaining is very real lol

    @Ranchu @rootofskynet I like him; he's friendly and very understanding of the personal things, but he doesn't seem to listen very well.
  • 3
    @Root
    With any Peter principled manchild manager, I've found you basically have to make the failure theirs before they'll even consider entertaining the idea they're wrong. Then they take credit for your work. They're so far our of their depth they can't see the light of day, so in their mind their entire job is putting out faerie fire. I feel for you. 🤦‍♀️
  • 4
    @Root I usualy use inception for this.

    Drop the idea a few times, and pretend to struggle with the task, then ask for help, until he "comes up" with a "new" idea. Then say "thats impossible".
    He will try and prove that "his" idea is the correct solution....

    You will not get credit - but with this type of a-hole manager, you never will.
  • 1
    This is so relatable. I don't know if its because of his ego or he secretly finds you professionally intimidating. Nevertheless, he should step out of himself and listen to your points more actively.

    You, on the other hand, can perfect your skills for emphasizing on points and standing your grounds when you know you're absolutely right.

    It maybe followed by a heated discussion once or twice, which will end with you being right, but in the end you will earn respect, and my ++
  • 1
    i remember we talked about this...

    Bloody christ.... That refactoring is like the cartoon scenes where a boat is leaking water and they realize that putting fingers and toes in the holes won't help, they'll still drown.

    Even IF you can plug all the places where this occurs, a fuckton of subtle regressions is likely.

    Dang.... :/ And that guy sounds like he needs to develop some brain.

    If someone would come up with something big as that, I would definitely thank him/her for being brave and having the guts to tackle that beast.
Add Comment