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

[12355] Remove discourse #12496

Conversation

isidzukuri
Copy link
Contributor

@isidzukuri isidzukuri commented May 18, 2024

What? Why?

eight years old. I think it was used with the Fair Food Forum which is now not used anymore. So we can take it out.

What should we test?

  • not logged in user should be able to view public pages of website
  • user should be able to login
  • logged in user should be able to view all pages of website

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

Looks good, Thanks @isidzukuri 🙏

I just need to confirm something, but otherwise it's good to go.

Comment on lines 19 to 35

## Link to the "Product Development - Backlog" item in Discourse
<!-- Put the link here, and put this epic link in the Discourse item as well for cross-referencing. -->
Add a custom footer
Pages 70
Home
Development environment setup

macOS (Sierra, HighSierra and Mojave)
OS X (El Capitan)
OS X (Mavericks)
Ubuntu
On Heroku
Rubocop
General guidelines

Spree Commerce customisation
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove this but I am not 100%, @mkllnk can you confirm ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe line 20 should be removed only

Copy link
Member

Choose a reason for hiding this comment

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

Actually this isn't related to the Discourse SSO integration.
We still use a Discourse forum for discussing new features (https://community.openfoodnetwork.org/), so this file shouldn't be changed at all.
(Well, actually it could be explained a lot better in this file, but I don't have suggestion for that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for review! addressed

@dacook dacook self-requested a review May 20, 2024 23:52
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Great, thanks for doing this one. There's just one change to revert, see comment.

Comment on lines 19 to 35

## Link to the "Product Development - Backlog" item in Discourse
<!-- Put the link here, and put this epic link in the Discourse item as well for cross-referencing. -->
Add a custom footer
Pages 70
Home
Development environment setup

macOS (Sierra, HighSierra and Mojave)
OS X (El Capitan)
OS X (Mavericks)
Ubuntu
On Heroku
Rubocop
General guidelines

Spree Commerce customisation
Copy link
Member

Choose a reason for hiding this comment

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

Actually this isn't related to the Discourse SSO integration.
We still use a Discourse forum for discussing new features (https://community.openfoodnetwork.org/), so this file shouldn't be changed at all.
(Well, actually it could be explained a lot better in this file, but I don't have suggestion for that)

@isidzukuri isidzukuri force-pushed the 12355-remove-unused-discourse branch from 2ec4c02 to b9a0c1c Compare May 21, 2024 06:02
@isidzukuri isidzukuri requested a review from dacook May 21, 2024 06:12
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Great, thank you for updating it! I didn't notice this before, but there's now a translation that's no longer used, so I've removed this as well.
Ready for testing!

@dacook dacook added the user facing changes Thes pull requests affect the user experience label May 21, 2024
@filipefurtad0 filipefurtad0 self-assigned this May 23, 2024
@filipefurtad0 filipefurtad0 added pr-staged-fr staging.coopcircuits.fr and removed pr-staged-fr staging.coopcircuits.fr labels May 23, 2024
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented May 23, 2024

Hey @isidzukuri ,
Welcome! 👋

I've tested access rights for:

  • guest user -> admin pages and /admin/general_settings/edit -> login screen
  • customer, logged in: account page -> 200; admin pages and /admin/general_settings/edit -> 401 Unauthorized
  • hub admin: admin pages -> 200 OK; /admin/general_settings/edit -> 401 Unauthorized
  • superadmin: visiting /admin/general_settings/edit and all other pages -> 200 OK

This corresponds to the expected results. I'd say we're good here!

Thank you for this contribution!
Merging.

@filipefurtad0 filipefurtad0 merged commit 88baf5e into openfoodfoundation:master May 23, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Remove unused Discourse SSO integration
4 participants