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

feat: allow orgs to not have a main contact if imported from an aggregator #10531

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

4nt0ineB
Copy link
Member

@4nt0ineB 4nt0ineB commented Jul 10, 2024

What

  • Added a script to remove orgs main contact that are not regular users. By default last migration, during the CRM integration, used the creator of the org to set the main contact. This caused the main contact to be agena3000/equadis/.. or even empty for some orgs.
  • Allow orgs to not have a main contact if it was imported from an aggregator such as agena3000 or equadis
  • New members can still be added to the orgs later ; it will be synced with the crm if the orgs has been accepted.

This script must be run in prod: scripts/migrations/2024_07_change_main_contact.pl

@4nt0ineB 4nt0ineB added 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers 🏭 Producers Platform - Odoo OdooCRM is used by the Open Food Facts team to manage producers (and more) relationships labels Jul 10, 2024
@4nt0ineB 4nt0ineB requested a review from a team as a code owner July 10, 2024 14:38
@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 49.25%. Comparing base (dc04d18) to head (73c16ad).
Report is 473 commits behind head on main.

Files Patch % Lines
lib/ProductOpener/Orgs.pm 0.00% 13 Missing ⚠️
lib/ProductOpener/CRM.pm 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10531      +/-   ##
==========================================
- Coverage   49.54%   49.25%   -0.29%     
==========================================
  Files          67       76       +9     
  Lines       20650    21868    +1218     
  Branches     4980     5238     +258     
==========================================
+ Hits        10231    10772     +541     
- Misses       9131     9774     +643     
- Partials     1288     1322      +34     

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

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

Some small points to maybe fix @4nt0ineB

I already approve though so you can merge when it's ok.

lib/ProductOpener/Orgs.pm Outdated Show resolved Hide resolved
use ProductOpener::Orgs qw/list_org_ids retrieve_org/;
use ProductOpener::Paths qw/%BASE_DIRS/;

my @not_users = qw(agena3000 equadis codeonline bayard database-usda countrybot);
Copy link
Member

Choose a reason for hiding this comment

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

Don't you want to add any @openfoodfacts.org user ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes good point.

Comment on lines 41 to 44
not defined $org_ref->{main_contact} # undefined
or $org_ref->{main_contact} =~ /^\s*$/ # empty or only whitespace
or $org_ref->{main_contact} =~ /[\p{Z}\p{C}]/ # contains unicode whitespace or control characters
or grep {$org_ref->{main_contact} eq $_} @not_users
Copy link
Member

Choose a reason for hiding this comment

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

It's better to parenthesis maybe (it's what we generally do, better be consistent)

Suggested change
not defined $org_ref->{main_contact} # undefined
or $org_ref->{main_contact} =~ /^\s*$/ # empty or only whitespace
or $org_ref->{main_contact} =~ /[\p{Z}\p{C}]/ # contains unicode whitespace or control characters
or grep {$org_ref->{main_contact} eq $_} @not_users
(not defined $org_ref->{main_contact}) # undefined
or ($org_ref->{main_contact} =~ /^\s*$/) # empty or only whitespace
or ($org_ref->{main_contact} =~ /[\p{Z}\p{C}]/) # contains unicode whitespace or control characters
or (grep {$org_ref->{main_contact} eq $_} @not_users)

)
{

$org_ref->{main_contact} = undef;
Copy link
Member

Choose a reason for hiding this comment

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

In the case of an empty contact, don't you wan't to see if there are people in the admins group ? (and maybe, just log for now, and report this to producers team).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah thanks, I completely forgot, that's what I wanted to do initially :)

@4nt0ineB 4nt0ineB merged commit ead1b0c into openfoodfacts:main Jul 11, 2024
11 checks passed
@4nt0ineB 4nt0ineB deleted the migrate-org-main-contact branch July 12, 2024 07:05
# and set it as main contact

my $admin = undef;
foreach my $admin_id (@{$org_ref->{admins}}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@{$org_ref->{admins} is a hash, changing to sort keys %{$org_ref->{admins}}

stephanegigandet pushed a commit that referenced this pull request Jul 23, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.38.0](v2.37.0...v2.38.0)
(2024-07-23)


### Features

* add new product event types for redis queue
([#10530](#10530))
([339dbc4](339dbc4))
* add product data to org table
([#10534](#10534))
([dce0518](dce0518))
* added a drag and drop zone in pro platform
([#10569](#10569))
([ce60b8f](ce60b8f))
* allow orgs to not have a main contact if imported from an aggregator
([#10531](#10531))
([ead1b0c](ead1b0c))
* launch the exports for all organizations that have the checkbox…
([#10561](#10561))
([c3aa2d1](c3aa2d1))
* Lint spaces in taxonomies
([#10563](#10563))
([c01cf91](c01cf91))
* make valid org filter field a dropdown
([#10524](#10524))
([b38e62c](b38e62c))
* measure and log duration of request and mongodb / off-query que…
([#10557](#10557))
([2cb1b1e](2cb1b1e))
* packaging add Ireland
([#10533](#10533))
([3f3196e](3f3196e)),
closes
[#1572](#1572)


### Bug Fixes

* add product_type to redis events
([#10550](#10550))
([6bd1c0e](6bd1c0e))
* added barcode-svg
([#10242](#10242))
([8da89da](8da89da))
* broken user space on pro platform
([#10541](#10541))
([b6e3017](b6e3017))
* change_main_contact.pl
([#10548](#10548))
([b2f90ea](b2f90ea))
* disable caching on pro platform
([#10516](#10516))
([4ccd714](4ccd714))
* GDSN import from Alnatura
([#10556](#10556))
([6e2673a](6e2673a))
* improve generated data for a .pl script
([#10532](#10532))
([1cab04c](1cab04c))
* pro_moderator_owner not stored for the admin/moderator user through
org/[orgid] facet
([#10560](#10560))
([e0441c6](e0441c6))
* rate limiter log config
([#10535](#10535))
([9a5168d](9a5168d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏭 Orgs 🏭 Producers Platform - Odoo OdooCRM is used by the Open Food Facts team to manage producers (and more) relationships 🏭 Producers Platform https://wiki.openfoodfacts.org/Platform_for_producers
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants