-
-
Notifications
You must be signed in to change notification settings - Fork 400
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: Able to view the list of users associated with an Organization #8511
feat: Able to view the list of users associated with an Organization #8511
Conversation
lib/ProductOpener/Display.pm
Outdated
@@ -3898,6 +3898,11 @@ HTML | |||
$user_template_data_ref->{edit_profile} = 1; | |||
$user_template_data_ref->{orgid} = $orgid; | |||
} | |||
my @org_members; | |||
foreach my $member_id (keys %{$user_or_org_ref->{members}}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this loop, you will need to load the user file that contains details such as the e-mail, language, country etc.
In Users.pm and Display.pm, we have code like this to do that:
`my $user_file = "$data_root/users/" . get_string_id_for_lang("no_language", $user_id) . ".sto";
if (-e $user_file) {
$user_ref = retrieve($user_file);`
It would be a good thing to create a retrieve_user($user_id) function in Users.pm to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay..doing it!
Thankyou so much <3
lib/ProductOpener/Display.pm
Outdated
@@ -3898,6 +3898,11 @@ HTML | |||
$user_template_data_ref->{edit_profile} = 1; | |||
$user_template_data_ref->{orgid} = $orgid; | |||
} | |||
my @org_members; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature should be available only to moderator of the pro platform, so we should have a condition on the value of $User{pro_moderator}, if the user is not a moderator, we don't create the structure with org members details.
I think it should work fine..only issues with the translatable string( make build_lang is not working in pro shell maybe)..Also, pls lmk when I have to add CSS to the table..will do that<3 |
For the table styling, we can use the same styles than we have for the packagings table when you edit a product: Currently we have specific styles for the packaging edition, but maybe we can make those styles more generic, and apply them to both the packaging edition and your table. `#packagings_components_edit_header { .packagings_components_edit_row { |
po/common/common.pot
Outdated
msgid "S.No" | ||
msgstr "S.No" | ||
|
||
msgctxt "member_userid" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have strings for "Name", "Email", "Country", "Language" etc. in the .po file, so we can reuse them without adding new translation strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okayy..will make the changes👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "member_userid" can you use the existing "username" string?
Co-authored-by: Stéphane Gigandet <stephane@openfoodfacts.org>
Co-authored-by: Stéphane Gigandet <stephane@openfoodfacts.org>
Thankyou so much...but the thing is I am not able to see the CSS changes in my localhost even after running make restart( tho not req but still)..not able to verify the CSS changes as it is not showing up :') |
I believe this is because the po_pro-dynamicfront that watches for scss changes is not started on the pro platform:
Short term: as this feature should also be accessible on the public platform (not only the pro platform), you could test it in a non-pro environment. Longer term: we need to fix the po_pro-dynamicfront bug. |
lib/ProductOpener/Display.pm
Outdated
if (defined $User{pro_moderator}) { | ||
my @org_members; | ||
foreach my $member_id (sort keys %{$user_or_org_ref->{members}}) { | ||
my $member_details = retrieve_user($member_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a convention, when a variable is an object reference, we suffix it with _ref. Coul you rename $member_details to $member_user_ref or similar?
</thead> | ||
[% SET count = 1 %] | ||
[% FOREACH users IN org_members %] | ||
<tbody id="org_table_body"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tbody element should be outside the FOREACH loop, otherwise you will have a table with more than 1 tbody.
[% FOREACH users IN org_members %] | ||
<tbody id="org_table_body"> | ||
<tr> | ||
<td data-cell =[% lang("serial_no") %]>[% count %].</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is "data-cell =[% lang("serial_no") %]" for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data-cell is a custom attribute used to store additional data or metadata related to the HTML element. This attribute is being used by CSS to manipulate the element for smaller screens--
td::before {
content:attr(data-cell) ": ";
float:left;
text-transform: capitalize;
font-weight: 600;
}
It would add : after the data cell value i.e serial no in this case when the screen width is small
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, thanks for the explanation.
scss/_off.scss
Outdated
@@ -1451,3 +1451,50 @@ a.panel_title { | |||
background-color: scale-color($button-bg-color, $lightness: -$button-function-factor / 2); | |||
color: #fff; | |||
} | |||
// organization members table in producers platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do it in a separate PR, but it would be a good thing to see if we can make the CSS code that we have for packagings generic, so that it can be applied to any table, instead of having similar separate CSS code for each table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes..It would be good to make the CSS generic for these tables👍...would open a separate PR for CSS things..will do it asap!
SonarCloud Quality Gate failed. 1 Bug No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you very much for the PR and incorporating all the feedback and suggestions!
What
Iterated over the keys of the
members
hash reference in$user_or_org_ref
and the reference to @org_members is assigned to theorg_members
key of the$user_template_data_ref
hash reference in order to access the list within the template.Screenshot
Currently the table is looking like this without CSS :') will try adding a generic CSS to the tables in a separate PR..for smaller screens the table contents are hidden from right.
Related issue(s) and discussion