Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Thank you so much, Sir! That really means a lot. It was such an awesome rabbit hole to trace from the outside.
Also, your reply really made my day!
Always happy to support the DEV Community!
Extends the existing ProfilePin model (previously User-only) to also support Organization as a profile_type, reusing the polymorphic pin/unpin pattern already used on individual user profiles.
ProfilePin now validates Organization as a valid profile_type and checks that a pinned article belongs to the organization (via organization_id) when the profile is an Organization.
New OrganizationProfilePinsController (slug-scoped, reuses OrganizationAdminScoped for authorization) handles create/destroy of organization pins. Only org admins (or site admins) can pin/unpin.
stories_controller#handle_organization_index now loads the organization's pinned articles separately from the regular chronological feed and excludes them from it, mirroring assign_user_stories for users.
organizations/_main_feed renders a dedicated "Pinned" section above the regular feed when pinned posts exist, reusing the same badge/styling as the user profile pinned section.
Added a "Pin to Organization" section to the post management page, visible to org admins managing an org-owned post, to pin/unpin from there.
Up to 5 pins per organization (same cap as user profiles); reordering is not implemented, matching parity with the existing user-profile pin feature which also orders purely by published_at.
Related Tickets & Documents
Related Issue #24057
Closes #24057
QA Instructions, Screenshots, Recordings
As an organization admin, go to /dashboard, open "Manage" on a post that belongs to your organization.
Under the new "Pin to Organization" section, click "Pin to Organization".
Visit the organization's page (/<org-slug>) — the post now appears in a "Pinned" card above the regular feed, and does not also appear in the regular feed below.
Pin up to 5 posts; a 6th pin attempt is rejected.
Unpin from either the "Manage" page; the post returns to the regular chronological feed.
A non-admin org member or an unaffiliated user cannot pin/unpin via POST /:slug/profile_pins (raises Pundit::NotAuthorizedError).
UI accessibility checklist
[x] Semantic HTML implemented? (reuses existing _main_feed/single_story markup and existing pin badge markup)
[x] Keyboard operability supported? (plain <button type="submit"> form controls)
[ ] Checked with axe DevTools and addressed Critical and Serious issues?
spec/requests/organization_profile_pins_spec.rb: create/destroy as org admin, five-pin cap, rejecting pins for articles outside the org, authorization failures for member/outsider/signed-out user.
spec/requests/stories_index_spec.rb: organization page renders the pinned section and does not duplicate the pinned post in the regular feed.
[optional] Are there any post deployment tasks we need to perform?
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
🐛 Plot twist: the bug wasn't the only thing I had to debug. My own test was wrong, too.
This started with four merged PRs and a missing badge. It turned into a dive through Forem's public code, a fix that got merged, and a code review that taught me why a passing test isn't always proof that your fix works.
Huge thanks to everyone who reviewed the PR and helped me catch what I'd missed!
Your turn: Have you ever had a test pass green while completely missing the bug it was supposed to catch? I'd love to hear that story. 👀
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Turns out there is a use case where if you change your primary email, Forem will not detect your commits. I made it where it counts your merged prs instead of your commits. Makes it much simpler and handles that unique case.
I have another open PR to refine it, since initially I thought it didn't work. Since it works, as of today, not sure if that will be closed or merge. We will see haha
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
That's great Sir @francistrdev, I actually saw your issue and PR both.😅
I'm kind of taking OSS contributions on forem way too seriously!😄
1 thing though, when you commit your changes to branch, the code rabbit and open hands review triggers by itself. I maybe wrong but I didn't have to call it by asking for full review.
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
The most valuable lesson here is that green tests don’t always mean correct code! 😅 Testing against the original code before celebrating a fix is such a simple but powerful habit. Loved how you turned a missing badge into a real open-source contribution! 👏
Really enjoyed your perspective! It's great connecting with people who care about writing reliable code. Hope we can keep in touch and share ideas along the way!
Absolutely! Passing tests doesn't always guarantee correct code. Great insight! 👏 I'd love to stay connected, exchange ideas, and keep in touch as friends!
French web developer mainly but touches everything. Volunteer admin mod here at DEV. I learn AI and Astro at this moment and databases. — Addict to Cappuccino and Music
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
French web developer mainly but touches everything. Volunteer admin mod here at DEV. I learn AI and Astro at this moment and databases. — Addict to Cappuccino and Music
The allow_multiple_awards: true detail is what makes the regression test meaningful. It removes the uniqueness validation as a safety net, so the test can actually expose duplicate creation on a second run. Good example of testing the failure mode, not just the expected outcome
Same class, from the other side. I published a fix for a 422 "Invalid authenticity token" on a scripted comment POST, then tested it by posting a comment. Green. Posted a second one to be sure. Green.
Then I ran the same recipe cold, fresh cookie jar, and it 422'd. The token I'd been grabbing off the article page belonged to a different form. It only ever worked because of state left over in a warm session.
The test wasn't wrong about the fix. It was wrong about the world. Same shape as your factory default: the thing that made it green was a default I didn't know I'd chosen.
Borrowed from your takeaway: for anything auth-shaped, "it passed" usually means "it passed with the session I happened to be holding." The regression run has to start from nobody. That's the git stash step, just for cookies.
Problem 1 is the one that sticks with me: a hardcoded constant quietly depending on a slug the database generated from a title. A cheap guard for DEV's own setup would be a spec or health check asserting that every key in BADGE_SLUGS resolves through Badge.id_for_slug. Then a renamed badge title fails CI loudly instead of switching off a whole badge tier for who knows how long. Debugging production from the public repo alone, with no DB access, is a seriously good skill to show off.
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
The allow_multiple_awards change is the part I'd turn into a reusable test habit: name the other mechanism that could make the assertion pass, then choose a fixture that removes that escape route. In this buggy movie-search exercise, I'd include both a published and an unpublished movie matching the same search. A broken matcher that returns nothing must not accidentally pass the filter test.
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Spot on—explicitly closing the 'escape route' in the test fixture so the assertion can only pass through the exact code path you're testing is the real game-changer!
Right. A second fixture with the alternate path deliberately unavailable would make the assertion easier to trust. You can then tell which behavior actually awarded the badge.
Self-taught tech enthusiast and founder of DeTLeng (www.detleng.com). Exploring AI, automation, Python, and web development. Not a professional coder, but a passionate builder and lifelong learner.
Excellent work, Dhruv! 👏 Debugging a real issue using only public code is impressive, but discovering why your first regression test passed is an even more valuable lesson.
A great reminder that green tests don't always mean correct tests. Thanks for sharing the journey so openly! 🚀
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
There's something pretty funny about earning your fifth merged PR by fixing the badge for your fourth 😄 The factory default was a sneaky one: the count was right, but the service was still attempting the duplicate. Nice example of why checking the final state alone can miss what went wrong.
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Lesson B deserves its systematic form: mutation testing is "run the spec against main" automated. Every surviving mutant is precisely a test that agrees with you - green for a reason other than the one you wrote it for. The cheaper habit that catches most of them is asserting on the mechanism, not only the outcome: your second-run test counted achievements, which the uniqueness validator also satisfies; counting create attempts would have separated first_or_create from silent validation failure.
Lesson A is the same rule as filtering a noisy error tracker: a warning that fires on expected configuration teaches operators to ignore the channel, and the cost is paid when a real one arrives. "warn means someone should act" has a corollary - on this install, with this config; expected absences are info by definition. And the OpenHands read-only lookup deserves a nod: confirming a falsifiable hypothesis against production data without mutating anything is the right instinct everywhere, not just in reviews.
Creator of GithubStarsManager — an open-source, local-first tool that organizes GitHub stars with AI: auto-categorization, semantic search, release tracking, and an MCP server for star library.
The Lesson B section is the one I will keep. A test that passes on main without the fix is not a regression test at all — it is just agreeing with the current behaviour, and the factory default hiding that is exactly the kind of quiet escape route that is easy to miss. The git stash check is cheap insurance, and your point about warn versus info lands too: a warning that fires on expected configuration just trains everyone to ignore warnings.
Growth at Tomosu AI. I write about what breaks when AI-written code ships: tests that pass for the wrong reasons, review gaps, and reliability checks before merge.
that moment when you realize the test passes because it hits a fixture instead of the pagination edge case is a classic. green CI gives you so much false confidence until the cron actually runs against the real list and quietly drops the tail.
The rerun test separates sequential idempotency from validation. A useful next case would be overlapping worker runs: pause both after the lookup finds no achievement, then let both continue and check the achievement count and notification count. Running the worker twice in sequence cannot exercise that overlap.
For badges that allow repeated awards, the deduplication key would need to identify this particular contribution award. A blanket unique constraint on (user_id, badge_id) would also remove the repeat-award feature.
One thing this reminded me of is how tricky idempotency gets once background jobs start triggering external side effects.
Take a recurring worker that awards badges and sends notifications. Preventing duplicate database records is only half the problem. Imagine two workers processing the same user concurrently, or a job successfully creating an achievement but crashing before the notification step completes. A retry can still produce inconsistent behavior even if the application-level uniqueness check looks correct.
This is why I like the distinction between application-level safeguards and database-level guarantees. In a high-concurrency system, I'd also look at transactional boundaries, unique constraints where the business rules allow them, and an outbox pattern for reliable event delivery.
Your example is a good reminder that a passing test doesn't necessarily prove the system behaves correctly under retries, partial failures, and concurrency.
I’m an SEO Specialist with 1+ year of hands-on experience in Search Engine Optimization, organic growth, content optimization, and digital marketing.
My expertise includes On-Page SEO, Technical SEO,
The part about the test passing for the wrong reason is such a good reminder. A green test doesn’t always mean the bug is fixed. Sometimes, the real learning comes from questioning what we think is already working. I really like it.👏
Full-Stack Developer building practical web apps with React, Node.js, Express, and PostgreSQL. Currently learning, building, and sharing along the way.
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
I think di, you should contribute, apparently we'll get to see how IITians solve issues, cause I've never met an IITian in my life.😅
Have A Great Day! and sorry for late comment!😅
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Thanks Bro!
I'm still learning ruby on rails. That's why my initial PRs were docs/front-end related and I made it to 4 merged PRs but didn't get the badge so today was the BUG SMASH DAY!😂
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Your git stash check is the habit I now wish I had adopted earlier: I once celebrated a green test that was really just the validator refusing a duplicate, so my fix was never exercised at all. Your log-level point lands the same way for me, because a warn that fires every hour stops being a signal and becomes background noise. Do you now run the fail-on-main check for every regression test, or only for the ones that touch uniqueness?
Upcoming Trainee Engineer @ Simform 🚀
Building, experimenting, and writing my way through tech.
AI • Web • Open Source • Challenges
If I find something interesting, I'll probably write about it. 😄
Location
Vadodara, Gujarat, India
Education
B.Tech in Computer Engineering @ Dharmsinh Desai University
Honestly, for every single regression test now! Taking 10 seconds to run git stash, watch the spec fail on main, and run git stash pop is the cheapest insurance policy against false positives—especially when factories have hidden defaults.
I would keep the stash check, but I would not trust it blindly for a new test, because git stash leaves untracked files behind by default. I once watched a spec pass on main for exactly that reason: the new factory file was still on disk, so the failure I expected never happened. Do you run the check with -u so new files are stashed too, or do you keep new files out of the regression check?
The allow_multiple_awards: true fixture is a great example of making the test invalidate the accidental guard that was hiding the bug. A useful review question for regression tests is: what unrelated validation or default could still make this assertion pass on the old code? Naming that false-positive path beside the test would make the intent even clearer for the next maintainer.
The first-test-was-wrong detail is the best part. Public-code debugging gets much stronger once the test proves it can observe the behavior you think it observes. Deliberately breaking the suspected rule before accepting a green repair is a habit worth keeping.
Digging into open-source Rails architectures like Forem usually exposes the exact operational bottleneck: unthrottled worker fan-out masquerading as asynchronous decoupling. When badge-evaluation hooks trigger on every write event, Redis queue depth spikes, pushing job latency from a predictable ~15ms to over 4.5s. That thrashing consumes an extra 2–4GB of volatile RAM in worker pools, triggering thread starvation and dropping transient execution state when connection pools to Postgres get saturated under load.
In low-latency production, the battle-tested fix is ditching per-event job dispatch for atomic bitmask checks or local cache lookups, draining badge criteria in batch sweeps to cut lock contention and eliminate worker churn. Did they end up accepting the eventual consistency trade-off to spare worker memory, or are they still paying the tax of real-time transactional evaluations?
Security engineer in Dublin. I spend my days on recon, small Python tools and shrinking attack surfaces. Learning SQLite internals; building a bookmark manager.
Thank you for this!
This bug has been around so long it had completely faded from everyone's consciounseness.
Thank you so much, Sir! That really means a lot. It was such an awesome rabbit hole to trace from the outside.
Also, your reply really made my day!
Always happy to support the DEV Community!
Have A Great Day!
What type of PR is this? (check all applicable)
Description
Extends the existing
ProfilePinmodel (previouslyUser-only) to also supportOrganizationas aprofile_type, reusing the polymorphic pin/unpin pattern already used on individual user profiles.ProfilePinnow validatesOrganizationas a validprofile_typeand checks that a pinned article belongs to the organization (viaorganization_id) when the profile is an Organization.OrganizationProfilePinsController(slug-scoped, reusesOrganizationAdminScopedfor authorization) handles create/destroy of organization pins. Only org admins (or site admins) can pin/unpin.stories_controller#handle_organization_indexnow loads the organization's pinned articles separately from the regular chronological feed and excludes them from it, mirroringassign_user_storiesfor users.organizations/_main_feedrenders a dedicated "Pinned" section above the regular feed when pinned posts exist, reusing the same badge/styling as the user profile pinned section.published_at.Related Tickets & Documents
QA Instructions, Screenshots, Recordings
/dashboard, open "Manage" on a post that belongs to your organization./<org-slug>) — the post now appears in a "Pinned" card above the regular feed, and does not also appear in the regular feed below.POST /:slug/profile_pins(raisesPundit::NotAuthorizedError).UI accessibility checklist
_main_feed/single_storymarkup and existing pin badge markup)<button type="submit">form controls)CriticalandSeriousissues?Added/updated tests?
spec/models/profile_pin_spec.rb: organization pin validity, ownership check, 5-pin cap.spec/requests/organization_profile_pins_spec.rb: create/destroy as org admin, five-pin cap, rejecting pins for articles outside the org, authorization failures for member/outsider/signed-out user.spec/requests/stories_index_spec.rb: organization page renders the pinned section and does not duplicate the pinned post in the regular feed.[optional] Are there any post deployment tasks we need to perform?
None.
Ok thanks
🐛 Plot twist: the bug wasn't the only thing I had to debug. My own test was wrong, too.
This started with four merged PRs and a missing badge. It turned into a dive through Forem's public code, a fix that got merged, and a code review that taught me why a passing test isn't always proof that your fix works.
Huge thanks to everyone who reviewed the PR and helped me catch what I'd missed!
Your turn: Have you ever had a test pass green while completely missing the bug it was supposed to catch? I'd love to hear that story. 👀
And Sir @francistrdev, I'd love a comment in here because I know you've contributed to forem before and skilled in ruby on rails.
Don't want to bother you, only when you're free!😅
I was wondering about this issue for a while. Thanks for fixing it!
@ben I know a lot of my PR's have been merged onto Forem and not received a badge for it, is it possible to fix that?
@dj29 Got it fixed lol
Turns out there is a use case where if you change your primary email, Forem will not detect your commits. I made it where it counts your merged prs instead of your commits. Makes it much simpler and handles that unique case.
I have another open PR to refine it, since initially I thought it didn't work. Since it works, as of today, not sure if that will be closed or merge. We will see haha
That's great Sir @francistrdev, I actually saw your issue and PR both.😅
I'm kind of taking OSS contributions on forem way too seriously!😄
1 thing though, when you commit your changes to branch, the code rabbit and open hands review triggers by itself. I maybe wrong but I didn't have to call it by asking for full review.
Again really great!
Have A Great Day!
I think the Open Hand review @ben would have to do it manually (triggering the flow to do so). Code Rabbit is given for every PR regardless lol.
You can do commands for Code Rabbit, which is nice. I notice everytime I fix a bug or a suggestion, they keep on recommending it.
Thats so awesome! Good job!
Thanks! Have A Great Day!
Great work! Nice contribution 👍
Thanks!😄
Have A Great Day!
The most valuable lesson here is that green tests don’t always mean correct code! 😅 Testing against the original code before celebrating a fix is such a simple but powerful habit. Loved how you turned a missing badge into a real open-source contribution! 👏
Really enjoyed your perspective! It's great connecting with people who care about writing reliable code. Hope we can keep in touch and share ideas along the way!
Absolutely! Passing tests doesn't always guarantee correct code. Great insight! 👏 I'd love to stay connected, exchange ideas, and keep in touch as friends!
Congrats!!
Thanks Sir!
Have A Great Day! and sorry for late comment!😅
thank you and good day!
The allow_multiple_awards: true detail is what makes the regression test meaningful. It removes the uniqueness validation as a safety net, so the test can actually expose duplicate creation on a second run. Good example of testing the failure mode, not just the expected outcome
Same class, from the other side. I published a fix for a 422 "Invalid authenticity token" on a scripted comment POST, then tested it by posting a comment. Green. Posted a second one to be sure. Green.
Then I ran the same recipe cold, fresh cookie jar, and it 422'd. The token I'd been grabbing off the article page belonged to a different form. It only ever worked because of state left over in a warm session.
The test wasn't wrong about the fix. It was wrong about the world. Same shape as your factory default: the thing that made it green was a default I didn't know I'd chosen.
Borrowed from your takeaway: for anything auth-shaped, "it passed" usually means "it passed with the session I happened to be holding." The regression run has to start from nobody. That's the
git stashstep, just for cookies.(Token-side writeup, if the details are useful: dev.to/kaj_ilands/your-scripted-po...)
Problem 1 is the one that sticks with me: a hardcoded constant quietly depending on a slug the database generated from a title. A cheap guard for DEV's own setup would be a spec or health check asserting that every key in BADGE_SLUGS resolves through Badge.id_for_slug. Then a renamed badge title fails CI loudly instead of switching off a whole badge tier for who knows how long. Debugging production from the public repo alone, with no DB access, is a seriously good skill to show off.
Thanks!
Have A Great Day! and sorry for late comment!😅
The allow_multiple_awards change is the part I'd turn into a reusable test habit: name the other mechanism that could make the assertion pass, then choose a fixture that removes that escape route. In this buggy movie-search exercise, I'd include both a published and an unpublished movie matching the same search. A broken matcher that returns nothing must not accidentally pass the filter test.
Spot on—explicitly closing the 'escape route' in the test fixture so the assertion can only pass through the exact code path you're testing is the real game-changer!
Right. A second fixture with the alternate path deliberately unavailable would make the assertion easier to trust. You can then tell which behavior actually awarded the badge.
Excellent work, Dhruv! 👏 Debugging a real issue using only public code is impressive, but discovering why your first regression test passed is an even more valuable lesson.
A great reminder that green tests don't always mean correct tests. Thanks for sharing the journey so openly! 🚀
Thanks Sir!
Have A Great Day! and sorry for late comment!😅
There's something pretty funny about earning your fifth merged PR by fixing the badge for your fourth 😄 The factory default was a sneaky one: the count was right, but the service was still attempting the duplicate. Nice example of why checking the final state alone can miss what went wrong.
Sure is, for whole morning I was like where's the badge?😅
And then I went like let me give my PR #5 a legendary arc.😄
But it was a really nice day!
Have A Great Day!
Lesson B deserves its systematic form: mutation testing is "run the spec against main" automated. Every surviving mutant is precisely a test that agrees with you - green for a reason other than the one you wrote it for. The cheaper habit that catches most of them is asserting on the mechanism, not only the outcome: your second-run test counted achievements, which the uniqueness validator also satisfies; counting create attempts would have separated first_or_create from silent validation failure.
Lesson A is the same rule as filtering a noisy error tracker: a warning that fires on expected configuration teaches operators to ignore the channel, and the cost is paid when a real one arrives. "warn means someone should act" has a corollary - on this install, with this config; expected absences are info by definition. And the OpenHands read-only lookup deserves a nod: confirming a falsifiable hypothesis against production data without mutating anything is the right instinct everywhere, not just in reviews.
The Lesson B section is the one I will keep. A test that passes on main without the fix is not a regression test at all — it is just agreeing with the current behaviour, and the factory default hiding that is exactly the kind of quiet escape route that is easy to miss. The git stash check is cheap insurance, and your point about warn versus info lands too: a warning that fires on expected configuration just trains everyone to ignore warnings.
that moment when you realize the test passes because it hits a fixture instead of the pagination edge case is a classic. green CI gives you so much false confidence until the cron actually runs against the real list and quietly drops the tail.
The rerun test separates sequential idempotency from validation. A useful next case would be overlapping worker runs: pause both after the lookup finds no achievement, then let both continue and check the achievement count and notification count. Running the worker twice in sequence cannot exercise that overlap.
For badges that allow repeated awards, the deduplication key would need to identify this particular contribution award. A blanket unique constraint on (user_id, badge_id) would also remove the repeat-award feature.
One thing this reminded me of is how tricky idempotency gets once background jobs start triggering external side effects.
Take a recurring worker that awards badges and sends notifications. Preventing duplicate database records is only half the problem. Imagine two workers processing the same user concurrently, or a job successfully creating an achievement but crashing before the notification step completes. A retry can still produce inconsistent behavior even if the application-level uniqueness check looks correct.
This is why I like the distinction between application-level safeguards and database-level guarantees. In a high-concurrency system, I'd also look at transactional boundaries, unique constraints where the business rules allow them, and an outbox pattern for reliable event delivery.
Your example is a good reminder that a passing test doesn't necessarily prove the system behaves correctly under retries, partial failures, and concurrency.
The part about the test passing for the wrong reason is such a good reminder. A green test doesn’t always mean the bug is fixed. Sometimes, the real learning comes from questioning what we think is already working. I really like it.👏
Nicely done 👏
Thanks!😄
Have A Great Day!
Nice! 👏🏻
Great work!
Great work! Nice contribution 💯👏🏻
This is really interestiing things that you cover
Thanks!
Have A Great Day! and sorry for late comment!😅
I should contribute too haha! Anyway, on behalf of the entire Dev Community, thank you so much for your contributions! ❤️🙌
I think di, you should contribute, apparently we'll get to see how IITians solve issues, cause I've never met an IITian in my life.😅
Have A Great Day! and sorry for late comment!😅
Bro, this is great! keep up the good work.
Thanks Bro!
I'm still learning ruby on rails. That's why my initial PRs were docs/front-end related and I made it to 4 merged PRs but didn't get the badge so today was the BUG SMASH DAY!😂
Great article!!!
Bro dropped the ultimate plot twist:'my first test was wrong.'🤯 Reading public source to fix a bug is next-level stuff. Great job!🚀❤👏🍀
Thanks! Glad you liked it!
Have A Great Day! and sorry for late comment!😅
Your git stash check is the habit I now wish I had adopted earlier: I once celebrated a green test that was really just the validator refusing a duplicate, so my fix was never exercised at all. Your log-level point lands the same way for me, because a warn that fires every hour stops being a signal and becomes background noise. Do you now run the fail-on-main check for every regression test, or only for the ones that touch uniqueness?
Honestly, for every single regression test now! Taking 10 seconds to run
git stash, watch the spec fail on main, and rungit stash popis the cheapest insurance policy against false positives—especially when factories have hidden defaults.I would keep the stash check, but I would not trust it blindly for a new test, because git stash leaves untracked files behind by default. I once watched a spec pass on main for exactly that reason: the new factory file was still on disk, so the failure I expected never happened. Do you run the check with -u so new files are stashed too, or do you keep new files out of the regression check?
The
allow_multiple_awards: truefixture is a great example of making the test invalidate the accidental guard that was hiding the bug. A useful review question for regression tests is: what unrelated validation or default could still make this assertion pass on the old code? Naming that false-positive path beside the test would make the intent even clearer for the next maintainer.The first-test-was-wrong detail is the best part. Public-code debugging gets much stronger once the test proves it can observe the behavior you think it observes. Deliberately breaking the suspected rule before accepting a green repair is a habit worth keeping.
Digging into open-source Rails architectures like Forem usually exposes the exact operational bottleneck: unthrottled worker fan-out masquerading as asynchronous decoupling. When badge-evaluation hooks trigger on every write event, Redis queue depth spikes, pushing job latency from a predictable ~15ms to over 4.5s. That thrashing consumes an extra 2–4GB of volatile RAM in worker pools, triggering thread starvation and dropping transient execution state when connection pools to Postgres get saturated under load.
In low-latency production, the battle-tested fix is ditching per-event job dispatch for atomic bitmask checks or local cache lookups, draining badge criteria in batch sweeps to cut lock contention and eliminate worker churn. Did they end up accepting the eventual consistency trade-off to spare worker memory, or are they still paying the tax of real-time transactional evaluations?
Bohdan Hordii // bohdanhordii.dev
Digging through public code is underrated. Nice work.