Skip to content

website/integrations: multiple integration edits#7923

Merged
tanberry merged 13 commits intogoauthentik:mainfrom
ZuluWhiskey:documentation-updates
Feb 23, 2024
Merged

website/integrations: multiple integration edits#7923
tanberry merged 13 commits intogoauthentik:mainfrom
ZuluWhiskey:documentation-updates

Conversation

@ZuluWhiskey
Copy link
Contributor

Details

  • This PR does not close an issues
  • After recently contributing the Immich documentation, I took a look at the other integrations within the Miscellaneous section of the website. I noticed that there were differences in documentation across the different services and thought it may be a good idea to try and update and standardise things a little. In this PR I've changed:
    • Home Assistant
      • I've tried to update formatting and also updated the guidance as I felt it was unclear
      • It was listed as Home-Assistant rather than Home Assistant, I've not seen it referenced with a hyphen elsewhere
    • FreshRSS
      • I've updated the authentik section only
    • Gravitee
      • I've updated the authentik and service sections
    • Immich (I spotted aspects in other services which I feel would improve this)
      • Added some notes
        I felt it best to not change too much in one PR before I get guidance from the project on whether this is wanted and/or useful

Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)
  • The translation files have been updated (make i18n-extract)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

@ZuluWhiskey ZuluWhiskey requested a review from a team as a code owner December 19, 2023 12:12
@netlify
Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 1f40103
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/65b378867d25b7000880622e
😎 Deploy Preview https://deploy-preview-7923--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Dec 19, 2023

Deploy Preview for authentik ready!

Name Link
🔨 Latest commit 1f40103
🔍 Latest deploy log https://app.netlify.com/sites/authentik/deploys/65b378860cd37c000861a171
😎 Deploy Preview https://deploy-preview-7923--authentik.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f9b998e) 92.61% compared to head (1f40103) 92.33%.
Report is 286 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7923      +/-   ##
==========================================
- Coverage   92.61%   92.33%   -0.29%     
==========================================
  Files         588      626      +38     
  Lines       29200    30922    +1722     
==========================================
+ Hits        27043    28551    +1508     
- Misses       2157     2371     +214     
Flag Coverage Δ
e2e 50.65% <ø> (+0.01%) ⬆️
integration 26.02% <ø> (+0.11%) ⬆️
unit 89.58% <ø> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
@ZuluWhiskey
Copy link
Contributor Author

I meant to say, if you'd prefer I chunk these PRs up per integration then let me know and I'll ensure to do so in future as I plan on making my way through the other integrations (as long as you're happy with what I'm doing of course!)

@rissson
Copy link
Member

rissson commented Dec 22, 2023

I meant to say, if you'd prefer I chunk these PRs up per integration then let me know and I'll ensure to do so in future as I plan on making my way through the other integrations (as long as you're happy with what I'm doing of course!)

I hope Jens agrees with me, but of course it's fine. It's absolutely great that you're taking the time to standardize our docs and we very much appreciate it!

Copy link
Member

@rissson rissson left a comment

Choose a reason for hiding this comment

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

I'm gonna go ahead and approve this, but I'd still like @tanberry to take a last look to make sure this is indeed the direction we want to go in

@rissson rissson requested a review from tanberry December 22, 2023 10:03
@ZuluWhiskey
Copy link
Contributor Author

Just to say, I'll try to keep working on these but will wait until there's confirmation on the approach I'm taking etc. 👍

@tanberry
Copy link
Contributor

Hi @ZuluWhiskey huge apologies for my taking so long to review! My bad. This is really a terrific PR, much appreciated indeed. I have noticed a lot of inconsistencies between the diff Integrations docs; we should probably create a template for new docs.

I made only one comment in your PR that needs your action... I think that on the Home Assistant one, we want to switch the order of sections so that authentik comes first.

Also, what are your thoughts about those sections in general? Does one always have to do configuration on the authentik side?

Again, thanks so much, this is 100% in the direction that we want to move with the Integrations docs: consistent ordering of the sections, headers using sentence-case capitalization, etc.

Copy link
Contributor

@tanberry tanberry left a comment

Choose a reason for hiding this comment

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

Thanks again! Just one requested change, on the Home Assistant one, to reorder the sections so that authentik is first. Everything else looks great!

@ZuluWhiskey
Copy link
Contributor Author

Hi @ZuluWhiskey huge apologies for my taking so long to review! My bad. This is really a terrific PR, much appreciated indeed. I have noticed a lot of inconsistencies between the diff Integrations docs; we should probably create a template for new docs.

Agreed with regards to the need for a template. Obviously one already exists but I think if we want consistency then ideally either we add more to the template or improve the guidance for writing documentation on the website?

Also, what are your thoughts about those sections in general? Does one always have to do configuration on the authentik side?

I would assume that authentik would always need to be configured in one way or another in order to work with a new integration. And I guess we have to consider that for some users, this may be their first time setting up anything within authentik so they may not have any pre-existing configuration.
In this vein however, I did notice when making these edits that there were consistent steps with only minor differences - For example the creation of an OAuth2/OpenID Provider. But I'm unsure that we could template this and apply it across the board 🤔

Again, thanks so much, this is 100% in the direction that we want to move with the Integrations docs: consistent ordering of the sections, headers using sentence-case capitalization, etc.

No worries at all, glad I can give something back to the project!

@tanberry tanberry merged commit 507f9b7 into goauthentik:main Feb 23, 2024
@tanberry
Copy link
Contributor

Thank you again @ZuluWhiskey sorry it took me so long!

kensternberg-authentik added a commit that referenced this pull request Feb 27, 2024
* main: (75 commits)
  Add missing commas, correction of spelling errors (#8680)
  website/docs: Add documentation for Glitchtip (#8182)
  website: add solve gitea group does not take effect (#8413)
  enterprise: fix read_only activating when no license is installed (#8697)
  core: fix blueprint export (#8695)
  web: bump the sentry group in /web with 1 update (#8687)
  web: bump yaml from 2.3.4 to 2.4.0 in /web (#8689)
  web: bump the eslint group in /web with 1 update (#8688)
  core: bump pytest from 8.0.1 to 8.0.2 (#8693)
  website: bump @types/react from 18.2.57 to 18.2.58 in /website (#8690)
  web: bump the eslint group in /tests/wdio with 1 update (#8691)
  core: bump sentry-sdk from 1.40.4 to 1.40.5 (#8692)
  core: bump coverage from 7.4.1 to 7.4.3 (#8694)
  providers/oauth2: fix inconsistent `sub` value when setting via mapping (#8677)
  translate: Updates for file locale/en/LC_MESSAGES/django.po in zh_CN (#8678)
  translate: Updates for file locale/en/LC_MESSAGES/django.po in zh-Hans (#8679)
  core, web: update translations (#8672)
  root: fix config loading after refactor during ruff migration (#8674)
  root: early spring clean for linting (#8498)
  website/integrations: multiple integration edits (#7923)
  ...
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.

4 participants