[allura] branch master updated: [#8471] wiki noindex: cleanup, tests, and check for comments now

[gc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gcruz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git


The following commit(s) were added to refs/heads/master by this push:
     new f0a4fe461 [#8471] wiki noindex: cleanup, tests, and check for comments now
f0a4fe461 is described below

commit f0a4fe4613874e8206e52f6ef61e94210fd9d700
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Tue Sep 27 11:32:18 2022 -0400

    [#8471] wiki noindex: cleanup, tests, and check for comments now
---
 Allura/allura/model/discuss.py        |  6 ++----
 ForgeWiki/forgewiki/tests/test_app.py | 23 ++++++++++++++++++++++-
 ForgeWiki/forgewiki/wiki_main.py      | 20 +++++++++++++-------
 3 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py
index a91ea93d7..49e0e4686 100644
--- a/Allura/allura/model/discuss.py
+++ b/Allura/allura/model/discuss.py
@@ -435,10 +435,8 @@ class Thread(Artifact, ActivityObject):
             q = q.limit(limit)
         return q
 
-    def find_posts(self, page=None, limit=None, timestamp=None,
-                   style='threaded'):
-        return self.query_posts(page=page, limit=limit,
-                                timestamp=timestamp, style=style).all()
+    def find_posts(self, *args, **kwargs):
+        return self.query_posts(*args, **kwargs).all()
 
     def url(self):
         # Can't use self.discussion because it might change during the req
diff --git a/ForgeWiki/forgewiki/tests/test_app.py b/ForgeWiki/forgewiki/tests/test_app.py
index 86ad91743..580423939 100644
--- a/ForgeWiki/forgewiki/tests/test_app.py
+++ b/ForgeWiki/forgewiki/tests/test_app.py
@@ -30,6 +30,7 @@ from allura import model as M
 from allura.tests import decorators as td
 from alluratest.controller import setup_basic_test, setup_global_objects
 from forgewiki import model as WM
+from forgewiki.wiki_main import ForgeWikiApp
 
 
 class TestBulkExport:
@@ -134,7 +135,7 @@ class TestApp:
     @td.with_wiki
     def setup_with_tools(self):
         self.project = M.Project.query.get(shortname='test')
-        self.wiki = self.project.app_instance('wiki')
+        self.wiki: ForgeWikiApp = self.project.app_instance('wiki')
         page = WM.Page.upsert('A New Hope')
         page.text = 'Star Wars Episode IV: A New Hope'
         page.mod_date = datetime.datetime(2013, 7, 5)
@@ -166,3 +167,23 @@ class TestApp:
         # c.app.uninstall(c.project) errors out, but works ok in test_uninstall for repo tools.  So instead:
         c.project.uninstall_app('wiki')
         assert not WM.Page.query.get(title='A New Hope')
+
+    def test_should_noindex_page(self):
+        assert not self.wiki.should_noindex_page(None)
+
+        assert not self.wiki.should_noindex_page(WM.Page.query.get(title='A New Hope'))
+
+        home_page = WM.Page.query.get(title='Home')
+        assert self.wiki.should_noindex_page(home_page)
+
+        home_page.text = 'changed'
+        home_page.commit()
+        assert not self.wiki.should_noindex_page(home_page)
+
+        page_default_text = WM.Page.upsert('old_default')
+        page_default_text.text = 'You can edit this description'
+        page_default_text.commit()
+        assert self.wiki.should_noindex_page(page_default_text)
+
+        page_default_text.discussion_thread.add_post(text='test comment')
+        assert not self.wiki.should_noindex_page(page_default_text)
diff --git a/ForgeWiki/forgewiki/wiki_main.py b/ForgeWiki/forgewiki/wiki_main.py
index 4a2e333b4..0c5791655 100644
--- a/ForgeWiki/forgewiki/wiki_main.py
+++ b/ForgeWiki/forgewiki/wiki_main.py
@@ -238,14 +238,20 @@ The wiki uses [Markdown](%s) syntax.
             return [
                 SitemapEntry(menu_id, '.')[SitemapEntry('Pages')[pages]]]
 
-    def should_noindex_page(self, page):
+    def should_noindex_page(self, page: WM.Page) -> bool:
         """Checks whether a page should not be indexed."""
-        # If page has default name (i.e. 'Home') and has not been edited, noindex.
-        res = page and page['title'] == self.default_root_page_name and page['version'] == 1
-        if not res:
-            if page and page['text'] in ('You can edit this description', ):
-                res = True
-        return res
+        if not page:
+            # this shouldn't happen; just a safeguard for using `page` below
+            return False
+        elif (page['title'] == self.default_root_page_name and page['version'] == 1) \
+                or page['text'] in ('You can edit this description', ):
+            # If page has default name (i.e. 'Home') and has not been edited, noindex.
+            # or old default text
+            # but not if comments are visible
+            visible_comments = page.discussion_thread.find_posts(status='ok', limit=1)
+            return not visible_comments
+        else:
+            return False
 
     def create_common_wiki_menu(self, has_create_access, admin_menu=False):
         links = []