Skip to content

Participant import rework - #6182

Open
Borreg0 wants to merge 54 commits into
OpenSlides:mainfrom
Borreg0:5124-participant-import-rework
Open

Participant import rework#6182
Borreg0 wants to merge 54 commits into
OpenSlides:mainfrom
Borreg0:5124-participant-import-rework

Conversation

@Borreg0

@Borreg0 Borreg0 commented May 12, 2026

Copy link
Copy Markdown
Contributor

closes #5124

@Borreg0 Borreg0 self-assigned this May 12, 2026
Javier Borrego added 26 commits May 22, 2026 14:25
@MSoeb

MSoeb commented Jul 7, 2026

Copy link
Copy Markdown

Talked with @Borreg0 and @rrenkert about it:

Some further adjustments have to be done. I've listed them in a file. Beware, it's written in german. ^^

PR 6182 - bereinigt.txt

@Borreg0
Borreg0 force-pushed the 5124-participant-import-rework branch from 05f9e93 to 46cbeba Compare July 17, 2026 12:45
@Borreg0
Borreg0 marked this pull request as ready for review July 27, 2026 11:33
@Borreg0 Borreg0 assigned MSoeb and unassigned Borreg0 Jul 27, 2026
@Borreg0
Borreg0 requested review from MSoeb and bastianjoel July 27, 2026 11:34

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove empty .spec.ts file

<os-backend-import-list
additionalInfo="{{
'Existing accounts can be reused or updated by using: <ul><li>Membership number <i>(recommended)</i><li>Username<li>Email address AND first name AND last name'
'Existing accounts can be reused or updated by using:<p>• Membership number <i>(recommended)</i><br>• Username<br>• Email address AND first name AND last name'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change seems unintended.

styleUrls: [`./head-bar.component.scss`],
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.Eager,
// changeDetection: ChangeDetectionStrategy.Eager,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very likely a bad idea to remove this.

>
@let entry = row.data;
@let type = column.type;
<div *osScrollingTableCellLabel>{{ getColumnLabel(column.property) }}</div>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not translated

@if (getHeader(def).type === 'boolean') {
<mat-checkbox disabled [checked]="item.value" />
}
&nbsp;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrap in span if possible.

Comment on lines +66 to +86
@Input()
public csvConfiguration: boolean;

@Output()
public selectedEncodingOutput = new EventEmitter<V>();

@Output()
public selectedColSepOutput = new EventEmitter<V>();

@Output()
public selectedTextSeparatorOutput = new EventEmitter<V>();

@Output()
public selectNewFile = new EventEmitter<Event>();

@Input()
public csvReloadButton: boolean;

@Input()
public csvReload: ParticipantImportService;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not want this in here.

Comment on lines +181 to +192
/**
* Summary adapted to the footer. Displays only "created", "updated", "referenced" and "error" columns.
*/
@Input()
public shortenedSummary: BackendImportSummary[];

/**
* Information to display the correct icons in footer
*/
@Input()
public GetSummaryInformation;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is also not supposed to be in here. This is a generic component. Specific things should be implemented here by inheritance or by utilizing another (maybe more flexible) component.

Comment on lines +121 to +126
/**
* Summary adapted to the footer. Displays only "created", "updated", "referenced" and "error" columns.
*/
@Input()
public shortenedSummary: BackendImportSummary[];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not supposed to be in here.

>
@if (showHeader && horizontalScroll) {
<div class="scrolling-table-header">
<ng-container [ngTemplateOutlet]="tableHeader"></ng-container>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't really see why you would use template outlets here.


public set labelTemplate(template: any) {
setTimeout(() => (this._labelTemplate = template));
this._labelTemplate = template;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We do not really know why this was done. When I suggested this change I asked you to further investigate. I can imagine that this could lead to problems in places where tables are statically defined and the labels container is not yet finished initializing. This might explain why you had to remove change detection Eager at some places. Please at the very least check usages in other places still work.

@bastianjoel bastianjoel assigned Borreg0 and unassigned bastianjoel and MSoeb Jul 27, 2026

@MSoeb MSoeb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Several changes needs to be done. Talked with @Borreg0 about it.

Here a list of next steps:
2026-07-30 - PR6182.txt

If you need an explanation please contact me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Participant Import: Rework

3 participants