Admin, Enterprise creation : set visibility to "Hidden" by default #11247

jibees · 2023-07-20T08:17:22Z

What? Why?

Closes Inconsistent default visibility setting, for newly created enterprises #9123

What should we test?

As an admin

As a admin, create an enterprise
Check that its visibility is set to only_through_links (ie. "Hidden" in the UI) by default

As a user

As a user, create an enterprise
Check that its visibility is set to only_through_links (ie. "Hidden" in the UI) by default

Release notes

Changelog Category: User facing changes

mkllnk

Excellent!

dacook

We have a minor inconsistency with the spec factory now, but it probably isn't worth changing 👍

dacook · 2023-07-21T05:09:39Z

db/schema.rb

@@ -233,7 +233,7 @@
    t.boolean "enable_subscriptions", default: false, null: false
    t.integer "business_address_id"
    t.boolean "show_customer_names_to_suppliers", default: false, null: false
-    t.string "visible", limit: 255, default: "public", null: false
+    t.string "visible", default: "only_through_links", null: false


I guess a later version of Rails doesn't specify a default limit anymore.
We could actually set a much smaller limit, but I don't know how much this helps optimise the db. Probably not worth it.

dacook · 2023-07-21T05:12:48Z

spec/factories/enterprise_factory.rb

@@ -14,6 +14,7 @@
    description { 'enterprise' }
    long_description { '<p>Hello, world!</p><p>This is a paragraph.</p>' }
    address
+    visible { 'public' }


I guess this is for backwards-compatibility with specs assuming this value?
If so, I think it would have been better to specify it in the specs themselves (they are now using a context with a non-default value). Perhaps the specs might need updating to test the default value too.

I guess this is for backwards-compatibility with specs assuming this value?

Yes.

If so, I think it would have been better to specify it in the specs themselves (they are now using a context with a non-default value).

I had hesitation yes ... I chose the easiest solution, but maybe not the best one... I will change that if you think it's better. Maybe will create a new trait for that.

filipefurtad0 · 2023-07-21T16:27:47Z

Hey @jibees ,

A very quick test - I'll have to explore this result a bit better - but after:

staging the PR
creating an enterprise (from an existing customer)

I've noticed there still is no option pre-set:

Checking the DB:

openfoodnetwork=> select visible from enterprises where id = 2520;
 visible 
---------
 t
(1 row)

I'll test other scenarios but it seems not to be doing what we'd expect - or maybe

jibees · 2023-07-24T08:22:26Z

Whoo. Super strange.

creating an enterprise (from an existing customer)

i've tested with the super-admin user I guess. What about you? Maybe this produce a difference?

filipefurtad0 · 2023-07-24T13:15:50Z

i've tested with the super-admin user I guess. What about you?

Indeed, creating an enterprise via superadmin (/enterprises, clicking the + New Enterprise button), works:

I was not explicit about this when the issue was opened, apologies - I did not consider this possibility, still learning about the app 🙈

I considered only, the case in which a user wishes to create its own enterprise. This process starts by clicking this tab, on the homepage, after logging in:

So, I'd propose to merge this PR, and leave the issue open, to account for enterprises created from users, sounds good @jibees ?

jibees · 2023-07-24T13:18:46Z

So, I'd propose to merge this PR, and leave the issue open, to account for enterprises created from users, sounds good @jibees ?

hummm ... I think I'd prefer manage this case in the same PR. Maybe there will be some interaction between them, and I rather treat within the same context. Back into In Dev then (if it's ok for you)

filipefurtad0 · 2023-07-24T13:35:53Z

Sure - that's good too. I've indicated the relevant spec and added a couple of lines to test the upcoming fix in this PR - hope this helps.

jibees · 2023-07-25T08:57:04Z

Back in Code Review since things have changed in the meantime. 3 more commits to do this.

+ update enterprise_factory.rb to create "public" enterprise through specs via its factory

dacook

Looks good! The defaults for creating an enterprise were different before, but now all the same:

Admin
Frontend ("register here")
API v0

I can't find any documentation for enterprises on API v0 so there's nothing to update ;)

drummer83 · 2023-07-27T10:36:18Z

Hi @jibees.
staging is failing on UK and AU (haven't tried FR). Could this be a problem with the migration, which may have run on those servers already?

Anyway, here are my notes BEFORE staging this PR for later reference.

Test scenarios

Enterprises can be created

as enterprise user
- on /register page (link in the footer banner)
- on /enterprises page (?)
as super admin user
- on /register page (link in the footer banner)
- on /enterprises page (?)
as new user (logged out)
- on /register page (link in the footer banner)
via API v0?

Enterprises should show/not show

on the /map page
on the /shops page
on the /producers page
as a reference in the profile of other enterprises

Database BEFORE staging

Check DB before staging the PR

Staging AU

openfoodnetwork=> SELECT visible, count (visible) FROM enterprises GROUP BY visible;
      visible       | count 
--------------------+-------
 t                  |     4
 public             |  1089
 only_through_links |  1368
 f                  |     2
(4 rows)

openfoodnetwork=> SELECT visible, id, name, email_address FROM enterprises WHERE visible = 't';
 visible |  id  |     name      |             email_address             
---------+------+---------------+---------------------------------------
 t       | 2515 | Producer 3    | producer3@kon***
 t       | 2518 | Producer 5 AU | kunde7@kon***
 t       | 2520 | tapas shop    | fil***
 t       | 2519 | Mario's farm  | mar***
(4 rows)


openfoodnetwork=> SELECT visible, id, name, email_address FROM enterprises WHERE visible = 'f';
 visible |  id  |     name      |           email_address           
---------+------+---------------+-----------------------------------
 f       | 2511 | an enterprise | fil***
 f       | 2516 | Producer 4 AU | producer4@kon***
(2 rows)

Staging UK

openfoodnetwork=> SELECT visible, count (visible) FROM enterprises GROUP BY visible;
      visible       | count 
--------------------+-------
 t                  |    14
 public             |   857
 only_through_links |   933
 f                  |     7
(4 rows)

openfoodnetwork=> SELECT visible, id, name, email_address FROM enterprises WHERE visible = 't';
 visible |  id  |            name             |               email_address               
---------+------+-----------------------------+-------------------------------------------
 t       | 1847 | Mario's legacy farm         | ***@gmail.com
 t       |    4 | accounts and billing        | x
 t       | 1786 | Cranberry Farm              | ***@gmail.com
 t       | 1838 | External Non-Producer Hub 1 | non-producer@kon***
 t       | 1823 | 976rfk                      | 
 t       | 1824 | place                       | ***@gmail.com
 t       | 1827 | Melissa McNab               | ***@gmail.com
 t       | 1841 | Non-Producer Profile 1      | profile@kon***
 t       | 1795 | profile only                | ***@gmail.com
 t       | 1819 | Ghanoush's place            | ***@protonmail.com
 t       | 1853 | Sconeshop                   | ***@gmail.com
 t       | 1854 | TestDFC                     | ***@protonmail.com
 t       | 1782 | Tree Hub                    | ***@protonmail.com
 t       | 1822 | Kimchi Hub                  | ***@gmail.com
(14 rows)

openfoodnetwork=> SELECT visible, id, name, email_address FROM enterprises WHERE visible = 'f';
 visible |  id  |              name               |            email_address            
---------+------+---------------------------------+-------------------------------------
 f       | 1842 | External Producer Shop 2        | kunde5@kon***
 f       | 1845 | enterprise                      | ***@gmail.com
 f       | 1849 | name                            | ***@gmail.com
 f       | 1851 | Producer 3 UK                   | producer3@kon***
 f       | 1839 | External Non-Producer Hub 2     | non-producer@kon***
 f       | 1840 | External Non-Producer Profile 1 | non-producer@kon***
 f       | 1825 | erg                             | ***@gmail.com
(7 rows)

After the PR the default in the DB should be 'only_through_links' (before it was 'public').

User interface BEFORE staging

The UI should have the "Not Visible" radio-button selected

I can confirm that for visible =t and visible = f in the DB no radio button is selected:

If you have any ideas regarding the failing staging process let me know please. Thanks!

filipefurtad0 · 2023-07-27T11:58:00Z

staging is failing on UK and AU (haven't tried FR). Could this be a problem with the migration, which may have run on those servers already?

Yes, it seems this was the case @drummer83 : after deleting the migration and re-deploying, it's good to test 👍

filipefurtad0 · 2023-07-27T17:24:29Z

Hey @jibees ,

Creating an enterprise as admin leads to the expected result

However, creating an enterprise from the homepage, still does not lead to the expected result:

Maybe there is something fishy with the PR / deployment?

I've just looked at the code at the staging server, and it seems that it does not have the most recent version - I've looked at the spec file:

This PR:

The staging server:

This is despite our message in devops channel:
https://openfoodnetwork.slack.com/archives/CEF14NU3V/p1690458870444589

And the success logs on GH actions:
https://github.com/openfoodfoundation/openfoodnetwork/actions/runs/5680130353/job/15393633414?pr=11247

Don't quite understand this result... 🤯

filipefurtad0 · 2023-07-27T17:36:20Z

Wait... Maybe something else got staged in the meantime? ping @Matt-Yorkley 😅

Uff - that's a relief. This looked way too mysterious for this time of the day. I'll try again later 😉

filipefurtad0 · 2023-07-28T11:15:28Z

New enterprises, created via:

superadmin

logging in as user:

Have the default hidden option selected 🎉

Previously created enterprises seem not to be changed by this PR / data migration:

I'd say this brings improvement and is good to ship.

We seem to have 4 options... while the UI only displays three:

openfoodnetwork=> SELECT visible, count (visible) FROM enterprises GROUP BY visible;
      visible       | count 
--------------------+-------
 only_through_links |   937
 t                  |    15
 public             |   856
 f                  |     7
(4 rows)

In staging-FR we have a 5th option:

openfoodnetwork=> SELECT visible, count (visible) FROM enterprises GROUP BY visible;
      visible       | count 
--------------------+-------
 only_through_links |  1177
 hidden             |     4
 t                  |    21
 public             |  1531
 f                  |    22
(5 rows)

The PR does not change this. I'd say we can do this DB cleanup elsewhere - please shout @jibees if you see this as a blocker for some reason. Merging!

jibees self-assigned this Jul 20, 2023

jibees force-pushed the 9123-inconsistent-default-visibility-setting-for-newly-created-enterprises branch from bbc748f to 0f10e60 Compare July 20, 2023 08:25

jibees changed the title ~~Admin, Enterprise creation : set visibility to Hide all references by default~~ Admin, Enterprise creation : set visibility to "Hidden" by default Jul 20, 2023

jibees force-pushed the 9123-inconsistent-default-visibility-setting-for-newly-created-enterprises branch 2 times, most recently from dd76a4e to 9c7a84c Compare July 20, 2023 09:32

mkllnk approved these changes Jul 20, 2023

View reviewed changes

dacook self-requested a review July 21, 2023 05:07

dacook approved these changes Jul 21, 2023

View reviewed changes

filipefurtad0 self-assigned this Jul 21, 2023

filipefurtad0 added the pr-staged-au staging.openfoodnetwork.org.au label Jul 21, 2023

filipefurtad0 removed the pr-staged-au staging.openfoodnetwork.org.au label Jul 21, 2023

filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label Jul 24, 2023

filipefurtad0 mentioned this pull request Jul 24, 2023

Adds pending assertion for #9123 #11267

Closed

filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jul 24, 2023

jibees and others added 4 commits July 25, 2023 10:57

Enterprises.visible attribute is set to only_through_links by default

7aed068

+ update enterprise_factory.rb to create "public" enterprise through specs via its factory

Set visible to only_through_links on enterprise registration

5a0d4d2

Set visible to only_through_links on enterprise creation

d22566a

Finally, update system test for an end-to-end testing

04f83fb

jibees force-pushed the 9123-inconsistent-default-visibility-setting-for-newly-created-enterprises branch from 7e915b8 to 04f83fb Compare July 25, 2023 08:57

jibees requested review from mkllnk and dacook July 25, 2023 09:02

dacook approved these changes Jul 25, 2023

View reviewed changes

mkllnk approved these changes Jul 26, 2023

View reviewed changes

drummer83 self-assigned this Jul 27, 2023

drummer83 added pr-staged-au staging.openfoodnetwork.org.au pr-staged-uk staging.openfoodnetwork.org.uk and removed pr-staged-au staging.openfoodnetwork.org.au pr-staged-uk staging.openfoodnetwork.org.uk labels Jul 27, 2023

drummer83 removed their assignment Jul 27, 2023

drummer83 added the feedback-needed label Jul 27, 2023

filipefurtad0 added no-staging-UK A tag which does not trigger deployments, indicating a server is being used pr-staged-uk staging.openfoodnetwork.org.uk and removed no-staging-UK A tag which does not trigger deployments, indicating a server is being used labels Jul 27, 2023

filipefurtad0 removed the feedback-needed label Jul 27, 2023

filipefurtad0 added feedback-needed and removed pr-staged-uk staging.openfoodnetwork.org.uk labels Jul 27, 2023

filipefurtad0 added pr-staged-uk staging.openfoodnetwork.org.uk pr-staged-fr staging.coopcircuits.fr and removed feedback-needed pr-staged-uk staging.openfoodnetwork.org.uk labels Jul 27, 2023

filipefurtad0 merged commit a975e2b into openfoodfoundation:master Jul 28, 2023
67 of 70 checks passed

filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Jul 28, 2023

filipefurtad0 mentioned this pull request Oct 4, 2023

[dev ENV only] Adds attribute to enterprise factory #11624

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin, Enterprise creation : set visibility to "Hidden" by default #11247

Admin, Enterprise creation : set visibility to "Hidden" by default #11247

jibees commented Jul 20, 2023 •

edited

mkllnk left a comment

dacook left a comment

dacook Jul 21, 2023

dacook Jul 21, 2023

jibees Jul 21, 2023

filipefurtad0 commented Jul 21, 2023

jibees commented Jul 24, 2023

filipefurtad0 commented Jul 24, 2023

jibees commented Jul 24, 2023

filipefurtad0 commented Jul 24, 2023 •

edited

jibees commented Jul 25, 2023

dacook left a comment

drummer83 commented Jul 27, 2023

filipefurtad0 commented Jul 27, 2023

filipefurtad0 commented Jul 27, 2023 •

edited

filipefurtad0 commented Jul 27, 2023

filipefurtad0 commented Jul 28, 2023

Admin, Enterprise creation : set visibility to "Hidden" by default #11247

Admin, Enterprise creation : set visibility to "Hidden" by default #11247

Conversation

jibees commented Jul 20, 2023 • edited

What? Why?

What should we test?

As an admin

As a user

Release notes

mkllnk left a comment

Choose a reason for hiding this comment

dacook left a comment

Choose a reason for hiding this comment

dacook Jul 21, 2023

Choose a reason for hiding this comment

dacook Jul 21, 2023

Choose a reason for hiding this comment

jibees Jul 21, 2023

Choose a reason for hiding this comment

filipefurtad0 commented Jul 21, 2023

jibees commented Jul 24, 2023

filipefurtad0 commented Jul 24, 2023

jibees commented Jul 24, 2023

filipefurtad0 commented Jul 24, 2023 • edited

jibees commented Jul 25, 2023

dacook left a comment

Choose a reason for hiding this comment

drummer83 commented Jul 27, 2023

Test scenarios

Database BEFORE staging

User interface BEFORE staging

filipefurtad0 commented Jul 27, 2023

filipefurtad0 commented Jul 27, 2023 • edited

filipefurtad0 commented Jul 27, 2023

filipefurtad0 commented Jul 28, 2023

jibees commented Jul 20, 2023 •

edited

filipefurtad0 commented Jul 24, 2023 •

edited

filipefurtad0 commented Jul 27, 2023 •

edited