Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#975 - Replace /content? endpoint by /content/scan? #976

Closed

Conversation

johannesloibl
Copy link
Contributor

Background: /content GET endpoint was blocked by our IT because it was causing a bug. Workaround: Use the /content/scan endpoint for searching pages. Since /scan does not support filtering for a page title, all pages have to be searched with pagination (sub-optimal, but our only option). See https://jira.atlassian.com/browse/CONFSERVER-57639

See issue #975

Background: /content GET endpoint was blocked by our IT because it was causing a bug.
Workaround: Use the /content/scan endpoint for searching pages. Since /scan does not support filtering for a page title,
all pages have to be searched with pagination (sub-optimal, but our only option).
See https://jira.atlassian.com/browse/CONFSERVER-57639
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i forgot this is another local fix i did. Feel free to cherry-pick this if you can reproduce the problem ;)

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to investigate this more and generate some unit testing around it. Anchor/reference management has not always been perfect in this extension in the past, and been trying to force the addition of checks to prevent any regressive that has been seen in a couple of releases.

If you wanted to re-submit in another pull request (with a DCO), I could try to extend off it with some tests (when I have the time to check some things). If you don't, no worries. I have noted the issue you have mentioned and will try to address this in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't update the PR then, just take the changes how you like ;)

Copy link
Member

Choose a reason for hiding this comment

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

@johannesloibl, by chance, would you happen to have a small sample document(s) showing this case? Just starting looking into it and have not yet been able to reproduce when creating multiple labels on heading and playing with a couple of reference scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could reproduce it like this:

Page Title
##########

.. _abc:

Foo
---

bla

:ref:`abc`

:ref:`abc`

:ref:`abc`

Bar
---

bla

Without the fix, the 3 references abc will result in a link suffix Foo.1, which is not existing, but Foo is.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any additional configuration entries/extensions used to produce this result? Trying a local test with the provided sample document does not appear to generate links with the mention suffix.

Output
[v1]
----

<h2>Foo</h2>
<p>bla</p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><h2>Bar</h2>
<p>bla</p>

[v2]
----

<h2><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">Foo</ac:parameter>
</ac:structured-macro><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">PageTitle-Foo</ac:parameter>
</ac:structured-macro>Foo</h2>
<p>bla</p><p><a href="#Foo">Foo</a></p><p><a href="#Foo">Foo</a></p><p><a href="#Foo">Foo</a></p><h2><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">Bar</ac:parameter>
</ac:structured-macro><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">PageTitle-Bar</ac:parameter>
</ac:structured-macro>Bar</h2>
<p>bla</p>

[report]
--------

(system)
 platform: Windows-10-10.0.19045-SP0
   python: 3.12.0 (tags/v3.12.0:0fb18b0, Oct 2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)]
   sphinx: 7.3.7
 docutils: 0.20.1
 requests: 2.31.0
  urllib3: 2.1.0
  builder: 2.6.0.dev0

(configuration)
confluence_editor: v2
confluence_publish: True
confluence_server_pass: (set)
confluence_server_url: https://(removed) (cloud)
confluence_server_user: (set)
confluence_space_key: (set; upper)

(confluence instance)
 connected: yes
   fetched: yes
   decoded: yes
    parsed: yes
      type: confluence
   version: 1000.0.0-b93331b30cee
     build: 6452

Copy link
Contributor Author

@johannesloibl johannesloibl Jun 25, 2024

Choose a reason for hiding this comment

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

good that you ask :D I just found it the problem only exists if i use the singleconfluence builder. The confluence builder works as expected!
Other than that i'm using this config:

"confluence_publish": True,
"confluence_server_url": ...,
"confluence_editor": "v1",
"singleconfluence_toctree": True,
"confluence_page_hierarchy": True,
"confluence_ignore_titlefix_on_index": False,
"confluence_publish_dryrun": False,
"confluence_page_generation_notice": True,
"confluence_publish_delay": 0.25,
"space_key": ...,
"parent_page": ...,
"publish_prefix": ...,
"confluence_request_session_override": <a function to b64 encode user pw for basic auth>,
"confluence_adv_ignore_nodes": ("PassthroughTextElement",)  # fix to work with sphinx-design

Copy link
Member

Choose a reason for hiding this comment

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

Applied a change (#1016) which should address issues when using singleconfluence. If willing to test out the development build and providing any feedback, it would be appreciated.

@jdknight
Copy link
Member

Closing since the main issue has been superseded by a change in #981.

@jdknight jdknight closed this Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants