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

Import Pages - csv example file improvement #4048

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

vkrasnovyd
Copy link
Contributor

Resolves #3822

  1. Shortened the example csv for importing participants and accounts to 1 line, as was shown in the images.
  2. Configured translating values in the selected columns to the current client language:
  • in 'gender' column for account-example
  • in 'gender' and 'groups' columns for participants-example

Notes in case at some stage example files are updated again:

  • If example files are extended with extra lines, the translation will work for all the example lines. No further adjustments will be needed.
  • Value in 'groups' column can be translated only if there's only one group name in a cell. Doesn't work for sample values like 'Delegates, Staff'.

@vkrasnovyd vkrasnovyd added enhancement General enhancement which is neither bug nor feature Schrödinger projectname labels Aug 22, 2024
@vkrasnovyd vkrasnovyd added this to the 4.2 milestone Aug 22, 2024
Copy link
Member

@bastianjoel bastianjoel left a comment

Choose a reason for hiding this comment

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

With custom translations it is possible to get a non injective set of genders. Therefore it might not be a good idea to allow users to use this. Especially with #4027 this will get much more complicated.

For groups this is a bit different. Those are translated in the backend and therefore guaranteed to be unique. Still the meetings language instead of the users language needs to be used there but I think it might be even better to use an actually existing group name from the groups repo.

@MSoeb please provide feedback on what you want here.

@MSoeb MSoeb removed their assignment Aug 30, 2024
@MSoeb MSoeb removed their request for review August 30, 2024 09:51
@rrenkert
Copy link
Member

With custom translations it is possible to get a non injective set of genders. Therefore it might not be a good idea to allow users to use this. Especially with #4027 this will get much more complicated.

For groups this is a bit different. Those are translated in the backend and therefore guaranteed to be unique. Still the meetings language instead of the users language needs to be used there but I think it might be even better to use an actually existing group name from the groups repo.

@MSoeb please provide feedback on what you want here.

Talked to @MSoeb: We do not want client side translations for genders in the example file. For groups the group name exisiting in the groups repo is the correct value to use.

@vkrasnovyd vkrasnovyd force-pushed the 3822-csv-example-file-improvement branch from a0968e5 to 0ae272e Compare September 4, 2024 08:03
@vkrasnovyd
Copy link
Contributor Author

As we discussed with @rrenkert, when the custom gender feature is finalized, I'm going to extend the export files with the gender option from the organization repo. Until then the issue should remain open.

@reiterl
Copy link
Member

reiterl commented Sep 5, 2024

Ah @vkrasnovyd you need to assign @bastianjoel , if you want his review.

Copy link
Member

@reiterl reiterl left a comment

Choose a reason for hiding this comment

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

Small remarks. Please wait for @bastianjoel review.

@vkrasnovyd
Copy link
Contributor Author

Ah @vkrasnovyd you need to assign @bastianjoel , if you want his review.

Thanks. I may still need to add a couple more changes based on your custom gender implementation. So until it's finalized, I'll keep it assigned to myself.

@Elblinator Elblinator assigned MSoeb and unassigned Elblinator and vkrasnovyd Oct 10, 2024
@Elblinator Elblinator requested a review from MSoeb October 10, 2024 12:02
@bastianjoel bastianjoel removed their assignment Oct 18, 2024
@Elblinator Elblinator requested a review from bspekker October 28, 2024 08:43
Copy link

@MSoeb MSoeb left a comment

Choose a reason for hiding this comment

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

One small adjustment needed:

  • Participant csv: The value for the 'new' column 'locked_out' is currently 'false'. This should be changed to value '0' to fit more the other attributes.

@MSoeb MSoeb assigned vkrasnovyd and unassigned bspekker and MSoeb Nov 18, 2024
@MSoeb MSoeb removed the request for review from bspekker November 18, 2024 10:56
Copy link

@MSoeb MSoeb left a comment

Choose a reason for hiding this comment

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

Looks good.

@MSoeb MSoeb removed their assignment Nov 18, 2024
@@ -32,7 +32,8 @@ export const getOrganizationSubscriptionConfig: SubscriptionConfigGenerator = ()
additionalFields: [`committee_ids`, `organization_tag_ids`],
follow: [
{ idField: `mediafile_ids`, fieldset: `organizationDetail` },
{ idField: `theme_id`, fieldset: DEFAULT_FIELDSET }
{ idField: `theme_id`, fieldset: DEFAULT_FIELDSET },
{ idField: `gender_ids`, fieldset: [`name`] }
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to subscribe this on the account/participant list.

@Elblinator Elblinator modified the milestones: 4.2, 4.3 Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement which is neither bug nor feature Schrödinger projectname
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Import Pages - csv example file improvement
8 participants