The Plan
A developer in my team, Isobel, was free to pick up some work, and she knew another team had too many items assigned, so their Team Lead contacted us to ask if we could pick up the work instead.
I wasn’t opposed to the idea, but we didn’t know what the enhancement was at the time, so didn’t know how long it would take and didn’t know if our Testers would be available to test it, or would the other team test it? It’s unclear, but their Team Lead wanted me to commit and accept reassigning it to us.
When I looked at the enhancement details, there was loads of confusion because it had loads of code changes linked to it, and was logged about a year and a half ago. It turns out, a year ago, a developer had done the work, checked the code in, but then got told to “roll it back” due to lack of testing resources. Then instead of going into the next release, it was somehow delayed a year… Now we have a lack of development and testing resources!
There was this comment too:
“The change will then need to go out as an urgent/emergency release. As the functionality is deemed potentially unsafe, we only have 7 days to get it out, it’s actually 6 now as it was logged yesterday.“
1.5 years later…
There was another item I was assigned recently, and that had similar chaos with how long it sat on our backlog
10th November 2020 - bug logged 15th September 2021 - assigned to the team
No wonder our users are often reluctant to report bugs because we don’t seem bothered about fixing them. Then our release process is also really long so sometimes there’s a 6 month lead time after we fix it.
I'll try and quickly explain the feature: Users can create these Templates which are composed of components. There's these special "calculation" components which use data added to the record and give you a score. Users can add data-entry components to the template which can be used by the calculation components. However, it's not clear which data is used, and we can change the calculation formula at any time; which makes it “unsafe”. You can also group several components together to make a Group Component. So the plan is basically to stop users from adding these calculator components, and they have to use our own Group Components which will have the calculator and the prerequisite data-entry components with it. For existing templates, we just have to show a message, telling the users their template isn’t recommended to be used.
I was invited to a meeting along with Isobel, and the managers tell us that all we need to do is take the old code, update some user-facing text which the UX team will confirm, then it just needs to be tested, but it should all work since it was ready for testing over a year ago. So in terms of development work, it sounds like we’d spend more time in meetings and generally discussing the work – than actually doing the work. (I later find out this is not true).
Assigned to Isobel
I tell Isobel to go through the changes and make sure they really do meet the requirements. A few days later, Isobel says she is on annual leave for 2 week but the changes are fine. The next day, I’m told I should look at it instead.
Assigned to Me
After an hour of testing it out, I find that there was:
- A control partially truncated
- Some extra spacing in one of the error messages
- Some components that were disabled that shouldn’t be
- Inconsistency in behaviour between “Templates” and “Templates Pro”
- Group Item logic was completely wrong
- Blank warnings were appearing for all other components
So how did the original developer think this was ready? Why did Isobel think it was ready?
So I start to fix the issues and I find copy-and-pasted code, redundant code, unclear code, code which could easily be unit tested but isn’t. I spent around 3 weeks sorting it out and it still wasn’t perfect. Meanwhile, I was invited to other meetings to say they changed their mind about some features. I had to undo some changes, change more UI text, and disable a few more components. In hindsight, I think I may as well have binned it, and started it again from scratch.
When I thought it was ready, I had the Pull Request created in the first week of January, ready to be checked-in for testing, however, there were no testers free, so it sat there for a month.
Eventually, the testers begin testing it and find a few problems with it. I fixed 3 out of 4 issues, but the last one seemed to be impossible to fix due to another bug which really needed to go to the specialist team that dealt with that area.
The actual template knows where it came from, but the Group item inside doesn’t. There was this interesting variable name that made me smile.
isOneOfOurs
I showed it to a colleague
“seems like Britain First wrote this”
Bants from colleague
There was some code where we set the originID to either the user’s or our organisationID. However, it set it to 0 which then assumed it was one of ours. I tried looking at one of the other properties which was a different type of ID; a GUID, but it was blank, so it was broken there too.
I couldn’t see a simple way to fix this. It would be far too risky for me to change, and I definitely didn’t have time. So I got told to abandon it and it would be reprioritised.
I think it was around 6 weeks later, it was assigned to another team. So it is now it’s with its 4th team, approaching 2 years later. Maybe we can call this a “pass the parcel” enhancement.
Assigned to Kumar
I was aware that the development re-started (along with some other requirement changes) when I saw a Pull Request for it. It was from Kumar, a developer in India that is absolutely rubbish. Not only that, it is quite hard to help him because his English is fairly poor. I also can’t tell if he is trolling, or if he really is that bad.
I would have thought that Kumar would have been told to speak to me about the work so I can “hand it over”, or at least he should have seen my name and comments on the item and shown some initiative and asked me about it. As it goes, this new change was something I already had fixed, it just wasn’t checked in. I could tell his fix wouldn’t work just by reading his changes. I message him telling him this.
He later responds with this conversation:
Kumar 10:11 Hi Mate 10:12 With the changes i have raised as a PR, I created a page as Group item and section as well with itself containing the components as Group as well as non Group items and it seems working . can i share you the screen mate ?
Me 10:14 so a user authored page/section Group item which contains one of the components - shows the message and a Officially-authored page/section Group item which contains one of the calculators - does not show the message
Kumar 10:14 Exactly mate Kumar 12:34 Hi Mate, Shall i comment out that the fix has completed the scenario we discussed here ?
I didn’t believe him, so I checked his code out, built it and tested it out myself. Obviously broken as expected.
Me 12:50 just trying it now and it looks broken to me got an Officially Authored Group Page and all the components have warnings next to them
Kumar 13:10 Ok dude, I will have a look on it. Kumar 13:31 Hi Mate, As u said it was showing up all the warning I might have not removed the Pages and section flag check for showing warnings will debug the code mate,
The next day
Kumar 07:12 Hi Mate Good Morning Mate
I look at the latest Pull Requests and see his new changes. Instead of taking my code that I know works, he has come up with his own solution. I think it might work, just harder to read.
Me 08:59 did you test my changes or just write that yourself?
Kumar 09:11 no mate i have took it from latest service branch of my team was everything fine mate ? i tested that locally and working fine
Me 09:13 it is similar to what I had done, just more lines of code
Kumar 09:14 haha, ok mate i will make the changes as suggested in the comments Thanks Mate
It’s not really funny. You just wasted a full day’s work because you didn’t just use my code.
I explained to him that this item is a bit of a nightmare since there’s multiple places you need to change due to “Templates” and “Templates Pro” which doesn’t share much code. Then there’s some existing bugs, and many different combinations of templates you can create. He doesn’t seem to test his work at all, so I think he had no chance of getting this working. I was trying to emphasise how much testing needs doing with every change to try and get him to put some effort in. Unit testing would help alleviate some of the manual testing. The next day…
Kumar 13:27 Hi mate, Good noon It is becoming a night mare as u said :joy:, i have completed with the unit test case also have fixed other area like Warnings were happening in the “Admin org” too. i have fixed that now. Kindly review and guide me to proceed further
Me 15:58 <send him picture> that should have the main yellow banner at the top shouldn't it?
Kumar 15:58 If it was a “Admin org” org, it should not have mate
Me 15:59 it's not
Kumar 15:59 Then it should have i believe haah, one more PR patch upcoming ..? :exploding_head: literally with this work
Me 16:17 I suggested 2 unit tests. You have only done 1. I think it is the other scenario where it doesn't work
Kumar 16:32 Ok , but if the method will not execute if the template was Group template mate, so do i need to do that as well ? in turn it returns empty
Me 16:40 isn't your requirement that it shows the message regardless if it is Officially-authored or User-authored
Kumar 16:41 yes, that is the requirement i will check on the code once again mate haah lil confusing
Although the overall work is confusing, this is one of the simplest parts of it: Show a message regardless of who created it. If the method is “returning empty” then that is the bug, it should return a message.
There was a line of code like
if (!configEnabled && activeSubscription)
and he changed it to
if ((!configEnabled && activeSubscription)
||
(configEnabled && activeSubscription))
so I wrote: “so just activeSubscription then“
Kumar: I am not able to get ur point here, kindly guide me
Me: “true or false” is always true isn’t it?
Kumar yes mate, so shall it be framed like this if ((configEnabled || !configEnabled) && activeSubscription)
NO! That still says “true or false”. I was trying to think of how I can write a response without telling him he doesn’t understand the very basics of programming. This is like Day 1 of learning how to write code.
Kumar: haah, I got it, just “activeSubscription” is enough isn’t it?
I was glad he seemed to understand in the end, because I was tempted to tell him to change his career…then he adds:
“correct me if I am wrong”
He has zero confidence.
The very next day he then tells me there is another requirement to remove the banner that he has been changing, so “is there any point carrying on?”. He sends me a screenshot of the requirement rather than giving me a link. He is definitely sent here to troll.
I’m sure he must have known about this, and I don’t know why he either:
- didn’t make the deletions first (it would reduce the amount of code and reduce confusion)
- not change any code he knew he was going to delete
So he made changes trying to fix a feature he knew he was going to remove. I had invested time reading the code, manually testing it, and all this back and forth communication. What a waste of my time.
Unassigned?
The good news is that he is leaving so he is going to be another company’s problem. The bad news is this enhancement is going to be reassigned to someone else.
I’ll probably write another blog about Kumar; I’ve got some more notes on previous development work he has done. If this enhancement ever goes live, maybe I will write a follow-up blog on it too.