Fix/geo location falsy check

[dhaval314]

Proposed change

Resolves #3891

Description

Fixed a falsy check in Chapter.save() and Event.save() where valid coordinates (0.0, 0.0) were treated as missing data. Previously, if not self.latitude would evaluate to True for 0.0, triggering unnecessary calls to the generate_geo_location service. I updated this to explicitly check if self.latitude is None.

Changes Made

• Backend: Updated backend/apps/owasp/models/chapter.py and event.py to use is None checks.
• Tests: Added regression tests (test_save_does_not_call_geo_location_on_zero_coords) to chapter_test.py and event_test.py to ensure 0.0 is respected as a valid coordinate.

Checklist

• Required: I followed the contributing workflow
• Required: I verified that my code works as intended and resolves the issue as described
• Required: I ran make check-test locally: all warnings addressed, tests passed
• I used AI for code, documentation, tests, or communication related to this PR

[coderabbitai]

Summary by CodeRabbit

• Bug Fixes

  • Enhanced coordinate validation for Chapter and Event locations. The system now properly distinguishes between explicitly missing coordinates and valid zero values, preventing unnecessary automatic location generation when coordinates are set to zero.

• Tests

  • Added new test cases validating that zero coordinate values (latitude and longitude at 0.0) do not trigger automatic location generation.

Walkthrough

Replaced falsy checks for latitude/longitude with explicit None checks in Chapter.save and Event.save so 0.0 coordinates no longer trigger geolocation regeneration. Added unit tests confirming save does not call generate_geo_location when latitude and longitude are 0.0.

Changes

Cohort / File(s)	Summary
Model Geo-Location Logic backend/apps/owasp/models/chapter.py, backend/apps/owasp/models/event.py	Changed condition from if not self.latitude or not self.longitude: to if self.latitude is None or self.longitude is None: so only None coordinates trigger generate_geo_location().
Test Coverage backend/tests/apps/owasp/models/chapter_test.py, backend/tests/apps/owasp/models/event_test.py	Added tests that patch generation methods and assert saving with latitude=0.0 and longitude=0.0 does not call generate_geo_location().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• kasya

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix/geo location falsy check' clearly and concisely describes the main change: fixing a falsy check for geo-location coordinates.
Description check	✅ Passed	The description is directly related to the changeset, explaining the issue fixed and the changes made to Chapter.save() and Event.save() with added tests.
Linked Issues check	✅ Passed	The PR fully addresses issue #3891 by replacing falsy checks with explicit 'is None' checks in both Chapter.save() and Event.save(), and adds corresponding regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the geo-location falsy check issue; no out-of-scope modifications are present in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@backend/tests/apps/owasp/models/chapter_test.py`:
- Around line 298-309: The test
test_save_does_not_call_geo_location_on_zero_coords is referencing self.chapter
which is undefined; replace that with a locally created Chapter instance (e.g.,
chapter = Chapter(...)) and use chapter.generate_geo_location, chapter.latitude,
chapter.longitude and chapter.save in the test so it mirrors other tests that
instantiate Chapter locally; ensure you pass any required constructor fields to
Chapter so the instance is valid before asserting mock_geo.assert_not_called().

In `@backend/tests/apps/owasp/models/event_test.py`:
- Around line 462-473: The test
test_save_does_not_call_geo_location_on_zero_coords incorrectly references
self.chapter and tests the wrong model; replace usage of self.chapter with an
Event instance (e.g., self.event or a locally created event) and patch the Event
instance's generate_geo_location method. Specifically, in TestEventSave update
the test to create or use the Event object under test (Event instance), set
event.latitude = 0.0 and event.longitude = 0.0, call event.save(), and assert
the patched generate_geo_location was not called (patch.object(event,
"generate_geo_location") or equivalent).

[cubic-dev-ai]

2 issues found across 5 files

Confidence score: 3/5

• Two high-severity test failures are likely: TestChapterModel in backend/tests/apps/owasp/models/chapter_test.py and TestEventSave in backend/tests/apps/owasp/models/event_test.py reference self.chapter without defining it, leading to AttributeError at runtime.
• This introduces concrete risk in the test suite and may block validation of the related model/event behavior changes, so the merge isn’t fully safe without fixing the tests.
• Pay close attention to backend/tests/apps/owasp/models/chapter_test.py and backend/tests/apps/owasp/models/event_test.py - undefined self.chapter in tests causes runtime failures.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="backend/tests/apps/owasp/models/chapter_test.py">

<violation number="1" location="backend/tests/apps/owasp/models/chapter_test.py:304">
P1: `self.chapter` is never defined on `TestChapterModel`. Every other test in this class creates a local `Chapter()` instance. This test will fail with `AttributeError` at runtime. Additionally, `BulkSaveModel.save` needs to be patched (as done in `test_save_method`) to prevent an actual database call.</violation>
</file>

<file name="backend/tests/apps/owasp/models/event_test.py">

<violation number="1" location="backend/tests/apps/owasp/models/event_test.py:468">
P1: Copy-paste bug: This test references `self.chapter` but the `TestEventSave` class has no `chapter` attribute. This test will raise `AttributeError` at runtime and never actually validate the fix for events. It should create an `Event` instance and test against that, similar to the other tests in this class.</violation>
</file>

---

Since this is your first cubic review, here's how it works:

• cubic automatically reviews your code and comments on bugs and improvements
• Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
• Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
• Ask questions if you need clarification on any suggestion

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: self.chapter is never defined on TestChapterModel. Every other test in this class creates a local Chapter() instance. This test will fail with AttributeError at runtime. Additionally, BulkSaveModel.save needs to be patched (as done in test_save_method) to prevent an actual database call.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/apps/owasp/models/chapter_test.py, line 304:

<comment>`self.chapter` is never defined on `TestChapterModel`. Every other test in this class creates a local `Chapter()` instance. This test will fail with `AttributeError` at runtime. Additionally, `BulkSaveModel.save` needs to be patched (as done in `test_save_method`) to prevent an actual database call.</comment>

<file context>
@@ -294,3 +294,16 @@ def test_update_data_no_save(self):
+        Ensure that 0.0 latitude and longitude do not trigger
+        unnecessary re-generation of geo-location data.
+        """
+        with patch.object(self.chapter, "generate_geo_location") as mock_geo:
+            self.chapter.latitude = 0.0
+            self.chapter.longitude = 0.0
</file context>

[arkid15r]

A cosplay code?

[dhaval314]

@arkid15r
My previous commit had some issues.

• Updated the unit tests to fix the AttributeError
• Ensured that the backend tests work locally

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[arkid15r]

Thanks for the contribution!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.39%. Comparing base (e987975) to head (2f30c7b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3896   +/-   ##
=======================================
  Coverage   95.38%   95.39%           
=======================================
  Files         463      463           
  Lines       14540    14540           
  Branches     2061     2017   -44     
=======================================
+ Hits        13869    13870    +1     
  Misses        328      328           
+ Partials      343      342    -1     

Flag	Coverage Δ
backend	95.67% <100.00%> (+<0.01%)	⬆️
frontend	94.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
backend/apps/owasp/models/chapter.py	100.00% <100.00%> (ø)
backend/apps/owasp/models/event.py	94.53% <100.00%> (+0.78%)	⬆️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e987975...2f30c7b. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dhaval314]

Thanks for the review! Happy to help.