Skip to content

Commit

Permalink
[IMP] hr_attendance_work_location_ip_check: remove redundant tests (r…
Browse files Browse the repository at this point in the history 
…elated to unused check ip allowed & validate location ip) and update according to get remote ip change
  • Loading branch information
@kongkea-aditi
kongkea-aditi committed Jan 27, 2025
1 parent 27b14d0 commit bae487e
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 138 deletions.
1 change: 0 additions & 1 deletion hr_attendance_work_location_ip_check/tests/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ def setUpClass(cls):
"work_location_id": cls.work_location.id,
"name": "Office Network",
"cidr": "192.168.1.0/24",
"sequence": 10,
"company_id": cls.env.company.id,
}
)
Expand Down
7 changes: 0 additions & 7 deletions hr_attendance_work_location_ip_check/tests/test_hr_attendance_cidr.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def test_01_cidr_validation(self):
"work_location_id": self.work_location.id,
"name": "Primary Network",
"cidr": "10.0.0.0/8",
"sequence": 20,
}
)
self.created_cidrs.append(cidr)
Expand All @@ -47,7 +46,6 @@ def test_02_multiple_cidrs(self):
"work_location_id": self.work_location.id,
"name": "Secondary Network",
"cidr": "10.0.0.0/8",
"sequence": 20,
}
)
self.created_cidrs.append(cidr)
Expand Down Expand Up @@ -83,7 +81,6 @@ def test_03_multi_company_cidrs(self):
"work_location_id": location2.id,
"name": "Company 2 Network",
"cidr": "172.16.0.0/12",
"sequence": 10,
"company_id": company2.id,
}
)
Expand Down Expand Up @@ -123,7 +120,6 @@ def test_05_cidr_sequence(self):
"work_location_id": self.work_location.id,
"name": "Specific Network",
"cidr": "172.16.0.0/24",
"sequence": 5, # Higher priority than existing
}
)
self.created_cidrs.append(cidr)
Expand All @@ -142,7 +138,6 @@ def test_06_overlapping_cidrs(self):
"work_location_id": self.work_location.id,
"name": "Overlapping Network",
"cidr": "192.168.1.128/25", # Overlaps with 192.168.1.0/24
"sequence": 30,
"active": True,
}
)
Expand All @@ -158,7 +153,6 @@ def test_07_cidr_cleanup(self):
"work_location_id": self.work_location.id,
"name": f"Test CIDR {i}",
"cidr": f"172.16.{i}.0/24",
"sequence": i,
}
)
cidrs.append(cidr)
Expand Down Expand Up @@ -192,7 +186,6 @@ def test_09_cidr_cleanup_error(self):
"work_location_id": self.work_location.id,
"name": "Test CIDR",
"cidr": "192.168.100.0/24",
"sequence": 30,
"company_id": self.env.company.id,
}
)
Expand Down
66 changes: 0 additions & 66 deletions hr_attendance_work_location_ip_check/tests/test_hr_attendance_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,75 +141,9 @@ def test_07_location_cidr_constraints(self):
location.write({"check_ip": True})
self.assertTrue(location.check_ip)

def test_08_ip_allowed_validation(self):
"""Test IP validation for work locations."""
location = self.work_location

# Test with valid IP
self.assertTrue(
location.check_ip_allowed("192.168.1.100"), "Should allow IP in CIDR range"
)

# Test with invalid IP
self.assertFalse(
location.check_ip_allowed("10.0.0.1"),
"Should not allow IP outside CIDR range",
)

# Test with invalid IP format
self.assertFalse(
location.check_ip_allowed("invalid_ip"), "Should handle invalid IP format"
)

def test_09_ip_check_required_global_disabled(self):
"""Test _is_ip_check_required when global IP check is off."""
self.env["ir.config_parameter"].sudo().set_param(
"hr_attendance_work_location_ip_check.ip_check_enabled", "False"
)
self.assertFalse(self.employee._is_ip_check_required())

def test_10_work_location_ip_check(self):
"""Test work location IP check with various scenarios."""
# Deactivate existing CIDRs first
existing_cidrs = self.work_location.allowed_cidr_ids
existing_cidrs.write({"active": False})

# Create new CIDR with different range
cidr = (
self.env["hr.work.location.cidr"]
.sudo()
.create(
{
"work_location_id": self.work_location.id,
"name": "Test CIDR",
"cidr": "10.0.0.0/24", # Different range to avoid overlap
"active": True,
}
)
)
self.created_cidrs.append(cidr)

# Test with active CIDR
self.assertTrue(self.work_location.check_ip_allowed("10.0.0.100"))

# Test with no active CIDRs
cidr.write({"active": False})
self.assertFalse(self.work_location.check_ip_allowed("10.0.0.100"))

# Test with invalid IP
self.assertFalse(self.work_location.check_ip_allowed("invalid_ip"))

# Reactivate CIDR and test again
cidr.write({"active": True})
self.assertTrue(self.work_location.check_ip_allowed("10.0.0.100"))

def test_11_work_location_ip_disabled(self):
"""Test check_ip_allowed method when check_ip is False"""
work_location = self.env["hr.work.location"].create(
{
"name": "Test Location no IP Check",
"address_id": self.test_partner.id,
"check_ip": False,
}
)
self.assertTrue(work_location.check_ip_allowed("192.168.1.100"))
19 changes: 0 additions & 19 deletions hr_attendance_work_location_ip_check/tests/test_hr_attendance_edge_cases.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,6 @@ def test_02_employee_ip_validation_errors(self):
with self.assertRaises(ValidationError):
self.employee._attendance_action_check("check_in")

def test_03_work_location_ip_validation(self):
"""Test work location IP validation edge cases."""
# Test with no active CIDRs
self.work_location.allowed_cidr_ids.write({"active": False})
self.assertFalse(self.work_location.check_ip_allowed("192.168.1.100"))

# Test with invalid IP format
self.assertFalse(self.work_location.check_ip_allowed("invalid_ip"))

# Test with empty IP
self.assertFalse(self.work_location.check_ip_allowed(""))

def test_04_cidr_range_validation(self):
"""Test CIDR range validation edge cases."""
# Create work location with a different CIDR range
Expand Down Expand Up @@ -246,10 +234,3 @@ def test_10_attendance_create_no_work_location(self):
)

self.assertTrue(attendance.exists())

def test_11_validate_location_ip_no_remote_ip(self):
"""Test _validate_location_ip with no remote IP"""
self.mock_ip.return_value = None # Simulate no IP
with self.assertRaises(ValidationError) as context:
self.env["hr.attendance"]._validate_location_ip(self.employee, "check_in")
self.assertIn("Unable to determine IP address", str(context.exception))
29 changes: 11 additions & 18 deletions hr_attendance_work_location_ip_check/tests/test_hr_attendance_ip.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ def setUpClass(cls):
"work_location_id": cls.work_location.id,
"name": "Office Network",
"cidr": "192.168.1.0/24",
"sequence": 10,
"company_id": cls.env.company.id,
}
)
Expand Down Expand Up @@ -186,14 +185,14 @@ def mock_get_remote_ip():
with self.assertRaises(ValidationError) as context:
self.employee._attendance_action_check("check_in")
self.assertIn(
"Unable to determine IP address for check_in operation",
"Unable to determine IP address for Check In operation",
str(context.exception),
)

def test_05_ip_validation_errors(self):
def test_05_ip_validation_false(self):
"""Test IP validation error handling"""
with self.assertRaises(ValidationError):
self.employee._is_ip_allowed("invalid-ip-format")
result = self.employee._is_ip_allowed("invalid-ip-format")
self.assertFalse(result)

def test_06_no_ip_error_message_direct_mock(self):
"""Test specific error message when IP cannot be determined with direct mock."""
Expand Down Expand Up @@ -321,21 +320,18 @@ def test_16_get_remote_ip_x_forwarded_for(self):
mock_request = Mock()
# Correctly set up side_effect to return values in sequence
mock_request.httprequest.headers.get.side_effect = [
None, # First call (X-Real-IP)
"192.168.1.100, 10.0.0.1", # Second call (X-Forwarded-For)
"192.168.1.100, 10.0.0.1", # First call (X-Real-IP)
None, # Second call (X-Forwarded-For)
]
mock_request.httprequest.remote_addr = "172.160.0.1"

with patch(f"{self.patch_path}.request", mock_request):
ip = self.employee._get_remote_ip()
self.assertEqual(ip, "192.168.1.100") # Correct assertion
# Assert that get was called with "X-Forwarded-For" and "X-Real-IP"
mock_request.httprequest.headers.get.assert_any_call("X-Real-IP")
mock_request.httprequest.headers.get.assert_any_call(
"X-Forwarded-For", None
)
# Assert that get was called with "X-Forwarded-For" first
mock_request.httprequest.headers.get.assert_any_call("X-Forwarded-For")
# Assert that get was called exactly twice
self.assertEqual(mock_request.httprequest.headers.get.call_count, 2)
self.assertEqual(mock_request.httprequest.headers.get.call_count, 1)
mock_request.httprequest.headers.get.reset_mock()

def test_17_get_remote_ip_x_real_ip(self):
Expand Down Expand Up @@ -363,11 +359,8 @@ def test_get_remote_ip_remote_addr(self):
ip = self.employee._get_remote_ip()
self.assertEqual(ip, "192.168.1.102")
# Verify the first call to get (for 'X-Forwarded-For')
mock_request.httprequest.headers.get.assert_any_call(
"X-Forwarded-For", None
)
# Verify the second call to get (for 'X-Real-IP')
mock_request.httprequest.headers.get.assert_any_call("X-Real-IP")
mock_request.httprequest.headers.get.assert_any_call("X-Forwarded-For")
mock_request.httprequest.headers.get.assert_called_with("X-Real-IP")
# Assert that get was called exactly twice
self.assertEqual(mock_request.httprequest.headers.get.call_count, 2)
mock_request.httprequest.headers.get.reset_mock()
Expand Down
26 changes: 0 additions & 26 deletions hr_attendance_work_location_ip_check/tests/test_hr_attendance_workflow.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,6 @@ def test_04_multi_attendance_validation(self):
self.env["hr.attendance"].create(vals_list)
self.assertIn("not allowed for", str(context.exception))

def test_05_validate_location_ip_direct(self):
"""Test direct validation of location IP."""
self.mock_ip.return_value = "192.168.1.100"

with self.with_user("[email protected]"):
# Test with valid IP
self.env["hr.attendance"]._validate_location_ip(self.employee, "check_in")

# Test with no work location
self.employee.work_location_id = False
self.env["hr.attendance"]._validate_location_ip(self.employee, "check_in")

# Test with disabled IP check
self.employee.work_location_id = self.work_location
self.work_location.check_ip = False
self.env["hr.attendance"]._validate_location_ip(self.employee, "check_in")

def test_06_attendance_state_transitions(self):
"""Test attendance state transitions and validations."""
self.mock_ip.return_value = "192.168.1.100"
Expand All @@ -166,15 +149,6 @@ def test_06_attendance_state_transitions(self):
str(context.exception),
)

def test_07_validate_location_ip_direct_no_allowed_ip(self):
"""Test direct validation of location IP."""
self.mock_ip.return_value = "10.0.0.1" # invalid IP

with self.assertRaises(ValidationError) as context:
# Test with invalid IP
self.env["hr.attendance"]._validate_location_ip(self.employee, "check_in")
self.assertIn("not allowed for", str(context.exception))

def test_07_attendance_batch_operations(self):
"""Test batch operations on attendance records."""
self.mock_ip.return_value = "192.168.1.100"
Expand Down
1 change: 0 additions & 1 deletion hr_attendance_work_location_ip_check/views/hr_work_location_views.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
context="{'default_work_location_id': active_id}"
>
<tree editable="bottom">
<field name="sequence" widget="handle" />
<field name="name" placeholder="e.g. Office Network" />
<field name="cidr" placeholder="e.g. 192.168.1.0/24" />
<field name="active" />
Expand Down

0 comments on commit bae487e

Please sign in to comment.