Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ New Features
- admin: Add log out button (`#870`_, bbaovanc)
- Add support for environment variables in config (`#1037`_, pkvach)
- Add Japanese localisation (`#1051`_, zurukumo)
- Allow access moderation queue (`#1028`_, gflohr)

.. _#870: https://github.com/posativ/isso/pull/870
.. _#966: https://github.com/posativ/isso/pull/966
Expand All @@ -24,6 +25,7 @@ New Features
.. _#1001: https://github.com/isso-comments/isso/pull/1001
.. _#1020: https://github.com/isso-comments/isso/pull/1020
.. _#1005: https://github.com/isso-comments/isso/pull/1005
.. _#1028: https://github.com/isso-comments/isso/pull/1028
.. _#1037: https://github.com/isso-comments/isso/pull/1037
.. _#1051: https://github.com/isso-comments/isso/pull/1051

Expand Down Expand Up @@ -57,6 +59,9 @@ Bugfixes & Improvements
- Python 3.12 support (`#1015`_, ix5)
- Disable Postbox submit button on click, enable after response (`#993`_, pkvach)
- Document title parameter and improve error handling for /new API (`#1058`_, pkvach)
- The `/latest` endpoint now has an optional parameter `mode`. The default value
of '1' retrieves published comments, the mode '2' retrieves posts waiting
moderation.

.. _#951: https://github.com/posativ/isso/pull/951
.. _#967: https://github.com/posativ/isso/pull/967
Expand Down
83 changes: 83 additions & 0 deletions isso/tests/test_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
import re
import tempfile
import unittest
import base64

from urllib.parse import urlencode

from werkzeug.wrappers import Response
from werkzeug.datastructures import Headers

from isso import Isso, core, config
from isso.utils import http
Expand Down Expand Up @@ -682,6 +684,37 @@ def testLatestOk(self):
self.assertIn(expected_text, reply['text'])
self.assertEqual(expected_uri, reply['uri'])

def testLatestWithMode(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

response = self.get('/latest?limit=5&mode=1')
self.assertEqual(response.status_code, 200)

body = loads(response.data)
expected_items = saved[-5:] # latest 5
for reply, expected in zip(body, expected_items):
expected_uri, expected_text = expected
self.assertIn(expected_text, reply['text'])
self.assertEqual(expected_uri, reply['uri'])

def testLatestWithInvalidMode(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

response = self.get('/latest?limit=5&mode=3')
self.assertEqual(response.status_code, 400)

def testLatestWithoutLimit(self):
response = self.get('/latest')
self.assertEqual(response.status_code, 400)
Expand All @@ -705,6 +738,20 @@ def testLatestNotEnabled(self):
response = self.get('/latest?limit=5')
self.assertEqual(response.status_code, 404)

def testPendingNotFound(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

response = self.get('/pending?limit=5')

# If the admin interface was not enabled we should get a 404.
self.assertEqual(response.status_code, 404)


class TestHostDependent(unittest.TestCase):

Expand Down Expand Up @@ -779,13 +826,17 @@ def setUp(self):
conf.set("moderation", "enabled", "true")
conf.set("guard", "enabled", "off")
conf.set("hash", "algorithm", "none")
conf.set("admin", "enabled", "true")
self.conf = conf

class App(Isso, core.Mixin):
pass

self.app = App(conf)
self.app.wsgi_app = FakeIP(self.app.wsgi_app, "192.168.1.1")
self.client = JSONClient(self.app, Response)
self.post = self.client.post
self.get = self.client.get

def tearDown(self):
os.unlink(self.path)
Expand Down Expand Up @@ -860,6 +911,38 @@ def testModerateComment(self):
# Comment should no longer exist
self.assertEqual(self.app.db.comments.get(id_), None)

def getAuthenticated(self, url, username, password):
credentials = f"{username}:{password}"
encoded_credentials = base64.b64encode(credentials.encode('utf-8')).decode('utf-8')
headers = Headers()
headers.add('Authorization', f'Basic {encoded_credentials}')

return self.client.get(url, headers=headers)

def testPendingPosts(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

password = "s3cr3t"
self.conf.set("admin", "enabled", "true")
self.conf.set("admin", "password", password)
self.conf.set("general", "latest-enabled", "true")
Comment thread
jelmer marked this conversation as resolved.
response = self.getAuthenticated('/latest?mode=2&limit=5', 'admin', password)
print(response.status)
self.assertEqual(response.status_code, 200)

body = loads(response.data)
expected_items = saved[-5:] # latest 5
for reply, expected in zip(body, expected_items):
expected_uri, expected_text = expected
self.assertIn(expected_text, reply['text'])
self.assertEqual(expected_uri, reply['uri'])


class TestUnsubscribe(unittest.TestCase):

Expand Down
57 changes: 54 additions & 3 deletions isso/views/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,33 @@ def get_uri_from_url(url):
return uri


def requires_auth(method):
def decorated(self, *args, **kwargs):
request = args[1]
auth = request.authorization
if not auth:
return Response(
"Unauthorized", 401,
{'WWW-Authenticate': 'Basic realm="Authentication Required"'})
if not self.check_auth(auth.username, auth.password):
return Response(
"Wrong username or password", 401,
{'WWW-Authenticate': 'Basic realm="Authentication Required"'})
return method(self, *args, **kwargs)
return decorated


def requires_admin(method):
def decorated(self, *args, **kwargs):
if not self.isso.conf.getboolean("admin", "enabled"):
return NotFound(
"Unavailable because 'admin' not enabled by site admin"
)

return method(self, *args, **kwargs)
return decorated


class API(object):

FIELDS = set(['id', 'parent', 'text', 'author', 'website',
Expand Down Expand Up @@ -1518,12 +1545,19 @@ def admin(self, env, req):
@apiName latest
@apiVersion 0.12.6
@apiDescription
Get the latest comments from the system, no matter which thread. Only available if `[general] latest-enabled` is set to `true` in server config.
Get the latest accepted comments from the system, no matter which thread. Only available if `[general] latest-enabled` is set to `true` in server config.
Comment thread
gflohr marked this conversation as resolved.
Outdated

@apiQuery {Number} limit
The quantity of last comments to retrieve

@apiExample {curl} Get the latest 5 comments
@apiQuery {Number{1,2}} [mode=1]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make these strings? it's hard to remember what these integers mean

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1028 (comment) suggested the numeric constants. But make a suggestion for string constants.

Copy link
Copy Markdown
Member

@jelmer jelmer Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mode=pending vs mode=accepted ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I will change that then.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed my mind after looking a little bit deeper into isso/views/comments.py. The mode property is a number in all API responses. Do you really want to change that to a string for requests?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open to the idea of supporting both integers and strings, but I do strongly feel that we should move away from integers.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that can only be done in requests. In the responses, it would break compatibility. So what should I do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to maintain full backwards compatibility would be to have mode still be a number, but an equivalent parameter state would expect strings. In the responses, both mode and string can then be given. And mode could be deprecated everywhere. But up to you, not my project.

The comments’ mode:
value | explanation
--- | ---
`1` | accepted: The comment was accepted by the server and is published.
`2` | in moderation queue: The comment was accepted by the server but awaits moderation.

@apiExample {curl} Get the latest 5 accepted comments
curl 'https://comments.example.com/latest?limit=5'

@apiUse commentResponse
Expand Down Expand Up @@ -1565,6 +1599,23 @@ def latest(self, environ, request):
"Unavailable because 'latest-enabled' not set by site admin"
)

mode = request.args.get('mode', "1")

if mode != "1" and mode != "2":
return BadRequest(
"Mode must either be '1' for accepted comments or '2' for pedning comments waiting moderation"
)

return self._latest(environ, request, mode)


def check_auth(self, username, password):
admin_password = self.isso.conf.get("admin", "password")

return username == 'admin' and password == admin_password


def _latest(self, environ, request, mode):
# get and check the limit
bad_limit_msg = "Query parameter 'limit' is mandatory (integer, >0)"
try:
Expand All @@ -1575,7 +1626,7 @@ def latest(self, environ, request):
return BadRequest(bad_limit_msg)

# retrieve the latest N comments from the DB
all_comments_gen = self.comments.fetchall(limit=None, order_by='created', mode='1')
all_comments_gen = self.comments.fetchall(limit=None, order_by='created', mode=mode)
comments = collections.deque(all_comments_gen, maxlen=limit)

# prepare a special set of fields (except text which is rendered specifically)
Expand Down
Loading