White Label: remove OFN banner from order confirmation emails when `hide_ofn_navigation` is activated for the shop #10744

jibees · 2023-04-25T08:38:53Z

⚠️ I did not put this feature behind the feature toggle white_label as we want it to be soon removed. That's why this PR should not be merge until we completely remove the white_label feature toggle.

~~But this is ready to be reviewed.~~

What? Why?

Closes Remove OFN header / banner in order confirmation emails #10556

BEFORE

AFTER

What should we test?

~~As a super-admin, activate white_label feature toggle for the instance~~
As a super-admin, activate hide_ofn_navigation for the shop in /admin/enterprises/ENTERPRISE_SLUG/edit#/white_label_panel

Check that order confirmation email does not contains the OFN header.

Release notes

Changelog Category: User facing changes

jibees · 2023-04-25T08:42:12Z

@kirstenalarsen I added screenshots before/after. Just one feedback if it looks right ;) Thanks! 🙏

dacook

Great, a nice simple addition built on top of the previous work.

dacook · 2023-04-26T03:59:38Z

spec/mailers/previews/subscription_mailer_preview.rb

+class SubscriptionMailerPreview < ActionMailer::Preview
+  def confirmation_email
+    SubscriptionMailer.confirmation_email(Spree::Order.complete.last)
+  end
+end


dacook · 2023-04-26T04:07:00Z

Putting back to 'in dev' because it doesn't need further review, but will need a rebase before it can be tested.

drummer83 · 2023-05-16T11:33:40Z

Does the email footer need adjustment as well?
Ping @kirstenalarsen

drummer83 · 2023-05-16T11:55:59Z

Unblocked, since #10824 was closed with #10825.

drummer83 · 2023-05-16T18:43:30Z

Hi @jibees,
Sorry, I don't know why but I still see the OFN header in the confirmation email.

This is the order confirmation email of a white labelled enterprise next to the order confirmation screen of the same order:

And this is the same for an order at a not-white-labelled enterprise:

For easier comparison you can see that both emails are identical in term of the banner:

I have tried this on staging UK and AU to exclude some mistake of mine. Both the same.
I'll move it back to In Dev. Let me know if I missed anything.

Thanks!

jibees · 2023-05-17T08:12:46Z

I (re)tested locally and works fine for me. Looks like something happens...

I redeployed the PR on staging-uk, and wanted to test the latest confirmation email sent on the browser via URL: https://staging.openfoodnetwork.org.uk/rails/mailers/subscription_mailer/confirmation_email (this URL to test confirmation email was introduced by this PR). I got a 404. So I suppose the PR hasn't been deployed on staging.

Will investigate...

jibees · 2023-05-17T08:32:48Z

I force pushed and give another try via semaphore. I tested URL, and 404 too.

Then, I decided to deploy through ansible script via ansible-playbook playbooks/deploy.yml --extra-vars "github_key=jibees:10556-remove-ofn-header-banner-in-order-confirmation-emails" --limit=uk-staging

PLAY RECAP **************************************************************************************************************************************************************************************
staging.openfoodnetwork.org.uk : ok=33   changed=20   unreachable=0    failed=0    skipped=6    rescued=0    ignored=0

But same conclusion, URL is not accessible.

jibees · 2023-05-17T08:48:51Z

oops, mailer previews aren't not available on staging for sure... 🤦

jibees · 2023-05-17T09:27:25Z

Thanks a lot @drummer83. I did not use the right mail... I used the subscription email "This order was automatically placed for you, and it has now been finalised. ...."

Should we also remove OFN banner on that one too @kirstenalarsen ?

…d order Available on: `/rails/mailers/subscription_mailer/confirmation_email`

… label + add specs that test for the presence of the footer logo which is in the header (I know it's a bit contradictory)

Available on `/rails/mailers/order_mailer/confirm_email_for_customer`

kirstenalarsen · 2023-05-18T03:33:30Z

yes please @jibees !

jibees · 2023-05-22T07:32:27Z

Back in code review then

dacook

Great!

drummer83 · 2023-05-23T23:13:20Z

Hi @jibees,
Thank you for this next part of white-labelling!

I compared some emails of a regular shopfront with a white-labelled one.

Left: regular shopfront; Right: white-labelled shopfront;
Order confirmation emails for customers (regular orders) is white-labelled. ✔️

Subscriptions: Order created email is not white-labelled. ❌

Subscriptions: Order confirmed email is white-labelled. ✔️

Other email, like the order confirmation emails for enterprise owners and sign up emails are not affected and still not white-labelled. ✔️

I think we can merge this one as it is a big step forward without any regressions. Probably the 'order placed' email needs to be white-labelled as well!? @kirstenalarsen
I didn't think of it either.

kirstenalarsen · 2023-05-24T21:18:31Z

Yes I think merge this and create another issue for the other emails. Then let's do the custom tab and I'll check the budget re. Completing these emails. Can you give me an estimate on the new issue please @jbees?

…

On Wed, 24 May 2023, 09:14 Konrad, ***@***.***> wrote: Merged #10744 <#10744> into master. — Reply to this email directly, view it on GitHub <#10744 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWGXSAWC3LN6X6BEDPKRKLXHVAFHANCNFSM6AAAAAAXKVJPRI> . You are receiving this because you were mentioned.Message ID: <openfoodfoundation/openfoodnetwork/pull/10744/issue_event/9321483003@ github.com>

jibees self-assigned this Apr 25, 2023

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch 2 times, most recently from b9c4d7a to 49ae06e Compare April 25, 2023 09:38

This was referenced Apr 25, 2023

[White Label] Add configuration into a single model/table instead of having many columns #10749

Open

White Label: allow hide of groups tab in shop navigation #10750

Merged

mkllnk approved these changes Apr 26, 2023

View reviewed changes

dacook approved these changes Apr 26, 2023

View reviewed changes

dacook added the blocked label Apr 26, 2023

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch from 49ae06e to 47e2a0d Compare April 26, 2023 13:58

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch 2 times, most recently from 18683de to 106a778 Compare May 15, 2023 15:46

drummer83 mentioned this pull request May 16, 2023

Activate white label feature #10825

Merged

drummer83 removed the blocked label May 16, 2023

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch from 106a778 to 3f044a8 Compare May 16, 2023 12:48

jibees marked this pull request as ready for review May 16, 2023 13:35

drummer83 self-assigned this May 16, 2023

drummer83 added pr-staged-au staging.openfoodnetwork.org.au pr-staged-uk staging.openfoodnetwork.org.uk labels May 16, 2023

drummer83 removed their assignment May 16, 2023

drummer83 removed pr-staged-uk staging.openfoodnetwork.org.uk pr-staged-au staging.openfoodnetwork.org.au labels May 16, 2023

jibees added the pr-staged-uk staging.openfoodnetwork.org.uk label May 17, 2023

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch from 3f044a8 to cdec3ab Compare May 17, 2023 08:16

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch from 5d8c33f to cdec3ab Compare May 17, 2023 08:49

jibees added 4 commits May 17, 2023 11:34

Create a subscription confirmation email preview of the last complete…

3ed89fc

…d order Available on: `/rails/mailers/subscription_mailer/confirmation_email`

Do not display OFN banner on subscription confirmation email if white…

f7d06fe

… label + add specs that test for the presence of the footer logo which is in the header (I know it's a bit contradictory)

Create an order confirmation email preview of the last completed order

dfafbfe

Available on `/rails/mailers/order_mailer/confirm_email_for_customer`

Do not display OFN banner on order confirmation email if white label

2336981

jibees force-pushed the 10556-remove-ofn-header-banner-in-order-confirmation-emails branch from 1522bac to 2336981 Compare May 17, 2023 09:35

jibees added feedback-needed and removed pr-staged-uk staging.openfoodnetwork.org.uk labels May 17, 2023

jibees removed the feedback-needed label May 22, 2023

sigmundpetersen requested review from dacook and mkllnk May 22, 2023 08:46

dacook approved these changes May 23, 2023

View reviewed changes

drummer83 self-assigned this May 23, 2023

drummer83 added the pr-staged-uk staging.openfoodnetwork.org.uk label May 23, 2023

drummer83 removed the pr-staged-uk staging.openfoodnetwork.org.uk label May 23, 2023

drummer83 merged commit 0854287 into openfoodfoundation:master May 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

White Label: remove OFN banner from order confirmation emails when `hide_ofn_navigation` is activated for the shop #10744

White Label: remove OFN banner from order confirmation emails when `hide_ofn_navigation` is activated for the shop #10744

jibees commented Apr 25, 2023 •

edited

Loading

jibees commented Apr 25, 2023

dacook left a comment

dacook Apr 26, 2023

dacook commented Apr 26, 2023 •

edited

Loading

drummer83 commented May 16, 2023

drummer83 commented May 16, 2023

drummer83 commented May 16, 2023

jibees commented May 17, 2023

jibees commented May 17, 2023 •

edited

Loading

jibees commented May 17, 2023 •

edited

Loading

jibees commented May 17, 2023 •

edited

Loading

kirstenalarsen commented May 18, 2023

jibees commented May 22, 2023

dacook left a comment

drummer83 commented May 23, 2023

kirstenalarsen commented May 24, 2023 via email

White Label: remove OFN banner from order confirmation emails when hide_ofn_navigation is activated for the shop #10744

White Label: remove OFN banner from order confirmation emails when hide_ofn_navigation is activated for the shop #10744

Conversation

jibees commented Apr 25, 2023 • edited Loading

What? Why?

BEFORE

AFTER

What should we test?

Release notes

jibees commented Apr 25, 2023

dacook left a comment

Choose a reason for hiding this comment

dacook Apr 26, 2023

Choose a reason for hiding this comment

dacook commented Apr 26, 2023 • edited Loading

drummer83 commented May 16, 2023

drummer83 commented May 16, 2023

drummer83 commented May 16, 2023

jibees commented May 17, 2023

jibees commented May 17, 2023 • edited Loading

jibees commented May 17, 2023 • edited Loading

jibees commented May 17, 2023 • edited Loading

kirstenalarsen commented May 18, 2023

jibees commented May 22, 2023

dacook left a comment

Choose a reason for hiding this comment

drummer83 commented May 23, 2023

kirstenalarsen commented May 24, 2023 via email

White Label: remove OFN banner from order confirmation emails when `hide_ofn_navigation` is activated for the shop #10744

White Label: remove OFN banner from order confirmation emails when `hide_ofn_navigation` is activated for the shop #10744

jibees commented Apr 25, 2023 •

edited

Loading

dacook commented Apr 26, 2023 •

edited

Loading

jibees commented May 17, 2023 •

edited

Loading

jibees commented May 17, 2023 •

edited

Loading

jibees commented May 17, 2023 •

edited

Loading