canonical / sphinx-docs-starter-pack

A documentation starter-pack
https://canonical-starter-pack.readthedocs-hosted.com/
Other
13 stars 35 forks source link

Integrate with Discourse #29

Open ru-fu opened 1 year ago

ru-fu commented 1 year ago

We have an "Ask a question on Discourse" link in the footer, which links to the start page of the configured Discourse.

Discourse supports adding a SHIM to a URL to identify a certain page and relate a forum post to it. We should use that in the link so that all comments for one page are combined into one post.

In addition, we should investigate if we have a way of displaying the content of the related Discourse post at the bottom of each page.

@evilnick can offer some insight.

pmatulis commented 1 year ago

We have an "Ask a question on Discourse" link in the footer, which links to the start page of the configured Discourse.

So a user would enable the link (to Discourse) if there is a pertinent Discourse for the subject of the guide?

Discourse supports adding a SHIM to a URL to identify a certain page and relate a forum post to it. We should use that in the link so that all comments for one page are combined into one post.

So have a Discourse topic for each RTD page? How would the topic be created? What Discourse category would be used?

In addition, we should investigate if we have a way of displaying the content of the related Discourse post at the bottom of each page.

Can you detail what this would look like? What would be the purpose of this?

ru-fu commented 1 year ago

From what I understood (quick discussion with Nick at the end), we would have a link to Discourse with a unique SHIM for each RTD page. The first time any user clicks on that link, a new Discourse post is created (probably only if the user actually writes and submits something). The next time any user clicks that link, they are directed to that post and can add a comment. No idea yet on where it will be created, need to investigate. :)

The same for displaying the contents. My first understanding is that if enabled (this should NOT be enabled by default!), any available comments from Discourse would be shown at the bottom of each RTD page. This would essentially give us commenting functionality (and with that community engagement) for RTD documentation.

evilnick commented 1 year ago

So, I have seen sites like this - https://blog.codinghorror.com/welcome-to-the-internet-of-compromised-things/#discourse-comments which add discourse via a simple iframe to the bottom of the page. there is a specific plugin for wordpress, but it is mentioned somewhere on the discourse site that this is sort of built-in behaviour. It is sort-of described here: https://meta.discourse.org/t/embed-discourse-comments-on-another-website-via-javascript/31963

It would be interesting to set up a simple set of published docs to see what may be possible

syncronize-issues-to-jira[bot] commented 1 year ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/DOCPR-57.

This message was autogenerated

akcano commented 6 months ago

OK, let's liven this up a bit. I did my research, and here are my current options, in order of increasing complexity estimations:

  1. Use the :discourse: metadata, as advertised by our very own Sphinx cheat sheet, to add the Discourse topic link under 'Related links' on the page. This is barely okay-ish UX-wise, requires pre-populating topics, keeping track of their IDs and placing directives manually. On the brighter side, it reuses existing mechanics and keeps intrusion to a minimum.

  2. Upgrade the 'Ask a question on Discourse' template part to do the same; it's marginally better and more idiomatic because of a more suitable link title that stays in the same place as before, but still requires pre-populating topics etc. This one's second best IMO.

  3. Use a Discourse feature that allows creating topics with a link; the downside here is that it doesn't redirect to existing topics, although it warns about duplicates. With minimal tinkering, I can add a Sphinx extension that generates a unique topic title for each page and issues a topic creation link if the topic is non-existent; if the topic already exists, the extension would issue a normal topic link accordingly. I think this hits the sweet spot but may seem a bit of an overkill compared to the previous two.

NB: Unfortunately, there's no counterpart to create posts under topics using a link; otherwise, this would solve all our problems.

  1. Now we're in Discourse API territory; didn't venture far here, but it would seemingly require stuff like API keys and a bit of extra logic on the front; I don't like the idea of this, mainly because it will probably be fun but may require unreasonable hours.

Speaking of embedding: long story short, this can be achieved, too, but I'd rather deal with the issue at hand first. Also, I'm not a fan of the idea to conflate Discourse UX with Sphinx-generated UX.

@ru-fu , @evilnick , what's your take on that?

ru-fu commented 6 months ago

Option (3) is the best choice imo. I would expect Discourse to do this automatically (check if a topic exists and if not create it), but if we can work around that with an extension, all good. :)

I don't see a need for opening the comment box automatically - we want users to actually read existing posts in the topic before adding their own comments, which they surely wouldn't do if they were brought right to the comment box.

And I think we should go for embedding the comments in the end, just to get closer to how we do documentation in Discourse, but I agree that is a second step after getting the link-up in place.

ru-fu commented 6 months ago

Also, FYI @evildmp

akcano commented 6 months ago

After trying a makeshift imitation of (3) out, I realized that it won't work nicely with private Discourse instances or whatever they are called. I'll try the API way.

akcano commented 4 months ago

@ru-fu Here's a PoC implementation: https://canonical-workshop--120.com.readthedocs.build/en/120/

The PR (let me know if you can't access it): https://github.com/canonical/workshop/pull/120

So far, the only downside I experienced is that it significantly slows down the build (currently, each page is queried).

ru-fu commented 4 months ago

In the PoC, are there any Discourse topics that exist already? I only saw pages with links to new-topic (didn't click through all pages though).

Overall, this looks good to me. It means though that if the title of a page is changed, it'll also link to a new Discourse topic. But this could make sense since it makes it easier to find the corresponding Discourse topic.

So far, the only downside I experienced is that it significantly slows down the build (currently, each page is queried).

How significant is it? Like, going from 3 minutes to 6 minutes, or to 3:10?

My first idea would be to add some caching so that instead of passing a function to "html-page-context", we store the result of the function in the extension and return the result from there instead of querying it more than once. But then, we are probably only querying each page once anyway - have you confirmed that?

s-makin commented 4 months ago

Would there be a way to disable this on PR builds?

keirthana commented 4 months ago

I am not sure if I understood this correctly but shouldn't this link back to an existing discourse topic where the user can leave comments?

Based on Ruth's explanation above, I expected to have a discourse post created and an option provided to me for commenting on the created discourse post. However, I got an option to create a new topic. (Apologies if I understood it wrong).

akcano commented 4 months ago

@ru-fu : there are no existing topics at this instance, although you can create one to see what happens. In short, when a topic with a given slug already exists, the extension generates a direct link to it (I looked for a 'post reply' mechanism, but it seems that'd require an API call).

It's quite simple to add an override mechanism to customise topic slugs per-page (would be almost the same as with the existing :discourse: variable, in fact) instead of simply deducing them from the page path as the extension does now.

The slowdown is noticeable, double time easily, which was quite unexpected for me (may depend on the connection, though). I spent some time looking for a workaround, but found nothing immediately usable. This alone will definitely need addressing before putting the extension to production use (perhaps, something akin to the requirements.txt generator we have in place, where we indeed can query a page one time and then continue with the result).


@s-makin : as any other extension, it can be enabled or disabled in custom_conf.py, albeit not on a per-PR basis. If needed, a toggle switch can be added to enable this feature explicitly, then we can tie the switch to run the extension with main builds (or RTD-based builds, for example) only.


@keirthana : currently, we're implementing item 3 from this comment, and I still think it's a good tradeoff after trying this approach and that.

Creating a dummy topic for each page in advance would cause overcrowding and confusion on Discourse. However, there is one point that definitely needs to be addressed, i.e. ensuring the page stays linked to a specific topic slug; as discussed in my response to Ruth above, it can be easily added if we're happy with the feature in general; then, it's up to the author to customise the topic slugs where needed (I don't expect topic slugs and pages to change or migrate too often).

akcano commented 4 months ago

I updated the PR; now, it only works for pages that have the :slug: variable set. If there's a page with this slug, the link points there; if there's none, the link creates a new topic with the given title. This provides some degree of flexibility to:

Of course, this is now basically the same feature we have with :discourse:, except that the link goes on the footer of the page and is kinda dynamic.

ru-fu commented 3 months ago

I feel we need to go back to the drawing board here ... there are several issues with the current approach:

Is there a way to handle this with JavaScript? This would consider the current state, and not the state at build time. It would also not have an impact on build times (we would need to have an eye on load times, but if we see an impact there, the script could be updated to run only when someone clicks on the link). And it should also be possible to accommodate for "hardcoded" slugs in the metadata, but they wouldn't need to be required.

akcano commented 3 months ago

@ru-fu I'm usually reluctant to resort to JS, if only for the reason some people may not have it enabled. However, your reservations are very valid, I also had similar concerns. Querying Discourse from the page itself will solve the build-time issue, obviuously, and we can still inject the hardcoded slugs if need be, but I'm not so sure about the mechanism that would reliably translate page titles into topic titles.

If we're back to the drawing board, I'll sleep on this maybe, keeping in mind that the starting idea was to provide improved UX, not complicating things for everyone :)

akcano commented 3 months ago

A brief update: while JavaScript approach is in theory feasible, it's plagued by CORS/CORB issues in practice. I'm looking for workarounds.

akcano commented 3 months ago

@ru-fu I tried some JavaScript workarounds for CORS/CORB, but to no success, unfortunately. As long as the documentation page and the Discourse topic are under different domains, the cross-site queries will stay problematic.

As a result, I suggest to either:

  1. Revert this improvement to a bare minimum:

    • no added default behaviour: 'Ask a question' generally points to the Discourse instance
    • when the :slug: variable is set (actual name can vary) on a page: it either points to an existing topic or suggests authoring a new one, decided at build time
    • synchronisation issues (creating a topic multiple times etc.) are ignored: if you control the link's availability via the :slug: variable, you can create the topic right after adding the slug to avoid a conflict. Far from perfect, but still good enough for a slow-growth environment (I'm personally OK with this outcome).
  2. Scrapping this improvement entirely. Having quality standards that prevent us from implementing a solution that we're not sure about is entirely OK.

akcano commented 3 months ago

On a somewhat orthogonal note.

Regarding the Discourse comments, I tried the native solution offered by Discourse (first mentioned by @evilnick earlier), but, being cross-origin in nature again, it needs to be enabled by both the Discourse admins, it seems, and generally looks a bit cumbersome to handle when you don't know the target Discourse instances in advance (i.e. not everyone will point to discourse.ubuntu.com for their discussions).

It's quite possible to parse Discourse topics for recent comments to output them in the native style of the page, and I think it's the safest bet.

ru-fu commented 3 months ago

I think there could be use for the slug variant you're suggesting. Let's discuss in Madrid and see if this is something teams would want to have implemented.