[change] Allow counters to return multiple replies #634 #647

pandafy · 2025-09-09T18:47:18Z

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.
I have updated the documentation.

Reference to Existing Issue

Closes #634

Blockers

[change] Change of Authorization: added replies, handled MaxQuotaReached, refactored #643 #645

pandafy · 2025-09-22T05:19:30Z

openwisp_radius/counters/base.py

    def get_reset_timestamps(self):
        try:
-            return resets[self.reset](self.user)
+            return resets[self.reset](self.user, self)


Passing the counter object to the reset allows flexibility to perform operation on user, group and group_check objects.

pandafy · 2025-09-22T05:27:53Z

@nemesifier can you suggest me how can I incorporate this implementation detail in https://openwisp.io/docs/dev/radius/user/enforcing_limits.html?

nemesifier · 2025-09-26T21:04:23Z

openwisp_radius/counters/base.py

+        # BACKWARD COMPATIBILITY: In previous versions of openwisp-radius,
+        # the Counter.reply_name was a string instead of a tuple. Thus,
+        # we need to convert it to a tuple if it's a string.
+        if not hasattr(self, "reply_name"):


AFAIK, getattr and hasattr have the same computing cost, so it's better to use getattr if we need to use the result later.

Suggested change

if not hasattr(self, "reply_name"):

reply_name = getattr(self, "reply_name", None)

if not reply_name:

nemesifier · 2025-09-26T21:05:14Z

openwisp_radius/counters/base.py

+            raise NotImplementedError(
+                "Counter classes must define 'reply_names' property."
+            )
+        if isinstance(self.reply_name, str):


Suggested change

if isinstance(self.reply_name, str):

if isinstance(reply_name, str):

nemesifier · 2025-09-26T21:05:33Z

openwisp_radius/counters/base.py

+                "Counter classes must define 'reply_names' property."
+            )
+        if isinstance(self.reply_name, str):
+            return (self.reply_name,)


Suggested change

return (self.reply_name,)

return (reply_name,)

nemesifier · 2025-09-26T21:06:25Z

openwisp_radius/counters/base.py

+            )
+        if isinstance(self.reply_name, str):
+            return (self.reply_name,)
+        return self.reply_name


Suggested change

return self.reply_name

return reply_name

openwisp_radius/counters/base.py

openwisp_radius/utils.py

nemesifier

There's a conflict, please update this PR.

Closes #634

coveralls · 2025-09-30T09:17:05Z

coverage: 98.717% (+0.004%) from 98.713%
when pulling 13c223a on issues/634-counters-return-multiple-values
into 340a64e on master.

nemesifier · 2025-09-30T17:29:54Z

.github/workflows/ci.yml

+      # TODO: Remove before merging
+      - issues/643-coa


Suggested change

# TODO: Remove before merging

- issues/643-coa

nemesifier · 2025-09-30T17:30:50Z

openwisp_radius/api/serializers.py

+            if consumed > int(obj.value):
+                consumed = int(obj.value)


Suggested change

if consumed > int(obj.value):

consumed = int(obj.value)

value = int(obj.value)

if consumed > value:

consumed = value

nemesifier · 2025-09-30T17:33:21Z

openwisp_radius/counters/base.py

        return row[0] or 0

-    def check(self, gigawords=gigawords):
+    def check(self, gigawords=True):


Suggested change

def check(self, gigawords=True):

def check(self):

nemesifier · 2025-09-30T17:34:50Z

openwisp_radius/counters/resets.py



-def _daily(user=None):
+def _daily(user=None, counter=None):


Suggested change

def _daily(user=None, counter=None):

def _daily(user=None, **kwargs):

nemesifier · 2025-09-30T17:35:01Z

openwisp_radius/counters/resets.py



-def _weekly(user=None):
+def _weekly(user=None, counter=None):


Suggested change

def _weekly(user=None, counter=None):

def _weekly(user=None, **kwargs):

nemesifier · 2025-09-30T17:35:09Z

openwisp_radius/counters/resets.py



-def _monthly(user=None):
+def _monthly(user=None, counter=None):


Suggested change

def _monthly(user=None, counter=None):

def _monthly(user=None, **kwargs):

nemesifier · 2025-09-30T17:35:36Z

openwisp_radius/counters/resets.py

    return _timestamp(start, end)


 def _monthly_subscription(user):


Suggested change

def _monthly_subscription(user, **kwargs):

nemesifier · 2025-09-30T17:35:47Z

openwisp_radius/counters/resets.py



-def _never(user=None):
+def _never(user=None, counter=None):


Suggested change

def _never(user=None, counter=None):

def _never(user=None, **kwargs):

pandafy marked this pull request as ready for review September 21, 2025 11:48

pandafy commented Sep 22, 2025

View reviewed changes

pandafy force-pushed the issues/634-counters-return-multiple-values branch 2 times, most recently from ab40470 to 2ddafa9 Compare September 23, 2025 11:10

pandafy self-assigned this Sep 24, 2025

nemesifier reviewed Sep 26, 2025

View reviewed changes

pandafy force-pushed the issues/634-counters-return-multiple-values branch from 2ddafa9 to b27a5b7 Compare September 29, 2025 15:16

Base automatically changed from issues/643-coa to master September 29, 2025 15:45

nemesifier reviewed Sep 29, 2025

View reviewed changes

pandafy added 6 commits September 30, 2025 14:35

[change] Allow counters to return multiple replies #634

ae5dfe6

Closes #634

[change] Updated counter resets to accept counter argument

15d7250

[fix] Minor changes

4d93726

[ci] Added build for issues/643-coa

c582c3e

[fix] Fixed RADIUS Usage API response

3fddf94

[req-changes] Improved BaseCounter.reply_names

d142269

pandafy force-pushed the issues/634-counters-return-multiple-values branch from b27a5b7 to d142269 Compare September 30, 2025 09:06

[test] Fixed coverage

369bea9

nemesifier added this to 25.09 Release Sep 30, 2025

github-project-automation bot moved this to Backlog in 25.09 Release Sep 30, 2025

nemesifier moved this from Backlog to In progress in 25.09 Release Sep 30, 2025

nemesifier reviewed Sep 30, 2025

View reviewed changes

[req-changes] Minor fixes

13c223a

pandafy force-pushed the issues/634-counters-return-multiple-values branch from e53b0a4 to 13c223a Compare September 30, 2025 18:35

nemesifier approved these changes Sep 30, 2025

View reviewed changes

nemesifier added the enhancement New feature or request label Sep 30, 2025

nemesifier added this to OpenWISP Priorities for next releases Sep 30, 2025

github-project-automation bot moved this to In progress in OpenWISP Priorities for next releases Sep 30, 2025

nemesifier merged commit 790d2e8 into master Sep 30, 2025
17 checks passed

nemesifier deleted the issues/634-counters-return-multiple-values branch September 30, 2025 20:58

github-project-automation bot moved this from In progress to Done in OpenWISP Priorities for next releases Sep 30, 2025

github-project-automation bot moved this from In progress to Done in 25.09 Release Sep 30, 2025

	if not hasattr(self, "reply_name"):
	reply_name = getattr(self, "reply_name", None)
	if not reply_name:

	if isinstance(self.reply_name, str):
	if isinstance(reply_name, str):

-            if consumed > int(obj.value):
-                consumed = int(obj.value)
+            value = int(obj.value)
+            if consumed > value:
+                consumed = value

	def _daily(user=None, counter=None):
	def _daily(user=None, **kwargs):



		def _weekly(user=None):
		def _weekly(user=None, counter=None):

	def _weekly(user=None, counter=None):
	def _weekly(user=None, **kwargs):



		def _monthly(user=None):
		def _monthly(user=None, counter=None):

	def _monthly(user=None, counter=None):
	def _monthly(user=None, **kwargs):

		return _timestamp(start, end)


		def _monthly_subscription(user):

	def _never(user=None, counter=None):
	def _never(user=None, **kwargs):

Uh oh!

[change] Allow counters to return multiple replies #634 #647

[change] Allow counters to return multiple replies #634 #647

Uh oh!

Conversation

pandafy commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reference to Existing Issue

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pandafy commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nemesifier left a comment

Choose a reason for hiding this comment

Uh oh!

coveralls commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pandafy commented Sep 9, 2025 •

edited

Loading

pandafy commented Sep 22, 2025 •

edited

Loading

coveralls commented Sep 30, 2025 •

edited

Loading