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

Update Sell page to use User Guide links from configuration #5636

Merged
merged 1 commit into from Jun 29, 2020
Merged

Update Sell page to use User Guide links from configuration #5636

merged 1 commit into from Jun 29, 2020

Conversation

fatihorhan
Copy link

What? Why?

We (OFN Turkey) translated the user guide to Turkish and changed the configuration to link to that page but Sell page still links to global user guide because it doesn't use the configuration.

https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/views/home/sell.html.haml has hard links for user guides but it should use the configuration like in https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/views/admin/shared/_user_guide_link.html.haml.

This PR fixes that.

What should we test?

Sell page link to the URL set in configuration.

Release notes

Sell page now uses configuration for user guide links.

Changelog Category: Fixed

Discourse thread

https://openfoodnetwork.slack.com/archives/C2GQ45KNU/p1592157142302400

@fatihorhan fatihorhan changed the title Update sell.html.haml Update sell to use User Guide links from configuration Jun 18, 2020
@fatihorhan fatihorhan changed the title Update sell to use User Guide links from configuration Update Sell page to use User Guide links from configuration Jun 18, 2020
@fatihorhan
Copy link
Author

Hello,
Build fails but I think it's unrelated to my changes. Should I rebase?

@sigmundpetersen
Copy link
Contributor

Hey @fatihorhan, yes this is a known "flaky" spec and unrelated to your PR.
I'm rebuilding to get it green, but nothing else you need to worry about.
We will fix the flaky spec in another issue/PR.

@fatihorhan
Copy link
Author

Thank you @sigmundpetersen .

Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

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

awesome, thanks for your PR!!!

Copy link
Contributor

@sauloperez sauloperez left a comment

Choose a reason for hiding this comment

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

Welcome and thanks for this meaningful contribution @fatihorhan 😍 !

@fatihorhan
Copy link
Author

Thanks for review and speedy approval.
Could this be merged? Is there something I need to do?

@sigmundpetersen
Copy link
Contributor

sigmundpetersen commented Jun 22, 2020

Hey @fatihorhan this will now be tested according to your testing notes in the PR description. Someone from our testing team will do this and if it passes it will then be merged and added to the next weekly release.

If you install the Zenhub browser extension you can follow the issues/PRs along our pipeline from Dev Ready through In Dev, Code Review and Test 👍

You can read more about the process here https://github.com/openfoodfoundation/openfoodnetwork/wiki/Pipeline-development-process

@filipefurtad0 filipefurtad0 self-assigned this Jun 29, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label Jun 29, 2020
@filipefurtad0
Copy link
Contributor

Hi @fatihorhan ,

I checked that changes set in the Configurations (/admin/contents/edit, as superadmin) take effect here on the /sell page, so the correct hyperlink on "Find out more in our user guide." can now be set.

Moving to ready to go,
Thank you for this PR!

@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jun 29, 2020
@Matt-Yorkley Matt-Yorkley merged commit bd0e4c7 into openfoodfoundation:master Jun 29, 2020
@fatihorhan fatihorhan deleted the make-user-guide-links-consistent branch July 14, 2020 16:31
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.

None yet

6 participants