Merge pull request #4351 from MaximilianKrambach/cleanup

cleanup of TODOS
This commit is contained in:
Maximilian Krambach 2019-04-16 13:24:38 +02:00 committed by GitHub
commit aec7280002
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 62 additions and 68 deletions

View File

@ -58,7 +58,6 @@ export abstract class BaseComponent {
/** /**
* Set the title in web browser using angulars TitleService * Set the title in web browser using angulars TitleService
* @param prefix The title prefix. Should be translated here. * @param prefix The title prefix. Should be translated here.
* TODO Might translate the prefix here?
*/ */
public setTitle(prefix: string): void { public setTitle(prefix: string): void {
const translatedPrefix = this.translate.instant(prefix); const translatedPrefix = this.translate.instant(prefix);

View File

@ -1,6 +1,7 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { LocalStorage } from '@ngx-pwa/local-storage'; import { LocalStorage } from '@ngx-pwa/local-storage';
import { Observable } from 'rxjs';
import { OpenSlidesStatusService } from './openslides-status.service'; import { OpenSlidesStatusService } from './openslides-status.service';
import { StoragelockService } from '../local-storage/storagelock.service'; import { StoragelockService } from '../local-storage/storagelock.service';
@ -31,7 +32,7 @@ export class StorageService {
public async set(key: string, item: any): Promise<void> { public async set(key: string, item: any): Promise<void> {
await this.lock.promise; await this.lock.promise;
this.assertNotHistroyMode(); this.assertNotHistoryMode();
if (item === null || item === undefined) { if (item === null || item === undefined) {
await this.remove(key); // You cannot do a setItem with null or undefined... await this.remove(key); // You cannot do a setItem with null or undefined...
} else { } else {
@ -44,16 +45,13 @@ export class StorageService {
/** /**
* get a value from the store. You need to subscribe to the request to retrieve the value. * get a value from the store. You need to subscribe to the request to retrieve the value.
* *
* TODO: This needs adjustment to ensure safe access.
* Since angular 7 `LocalStorrage.getItem` will return "unknown" instead of any.
* https://github.com/cyrilletuzi/angular-async-local-storage/blob/master/docs/MIGRATION_TO_V7.md
* @param key The key to get the value from * @param key The key to get the value from
* @returns The requested value to the key * @returns The requested value to the key
*/ */
public async get<T>(key: string): Promise<T> { public async get<T>(key: string): Promise<T> {
await this.lock.promise; await this.lock.promise;
return await this.localStorage.getUnsafeItem<T>(key).toPromise(); return ((await this.localStorage.getItem<T>(key)) as Observable<T>).toPromise();
} }
/** /**
@ -63,7 +61,7 @@ export class StorageService {
public async remove(key: string): Promise<void> { public async remove(key: string): Promise<void> {
await this.lock.promise; await this.lock.promise;
this.assertNotHistroyMode(); this.assertNotHistoryMode();
if (!(await this.localStorage.removeItem(key).toPromise())) { if (!(await this.localStorage.removeItem(key).toPromise())) {
throw new Error('Could not delete the item.'); throw new Error('Could not delete the item.');
} }
@ -75,7 +73,7 @@ export class StorageService {
public async clear(): Promise<void> { public async clear(): Promise<void> {
await this.lock.promise; await this.lock.promise;
this.assertNotHistroyMode(); this.assertNotHistoryMode();
if (!(await this.localStorage.clear().toPromise())) { if (!(await this.localStorage.clear().toPromise())) {
throw new Error('Could not clear the storage.'); throw new Error('Could not clear the storage.');
} }
@ -84,7 +82,7 @@ export class StorageService {
/** /**
* Throws an error, if we are in history mode. * Throws an error, if we are in history mode.
*/ */
private assertNotHistroyMode(): void { private assertNotHistoryMode(): void {
if (this.OSStatus.isInHistoryMode) { if (this.OSStatus.isInHistoryMode) {
throw new Error('You cannot use the storageService in histroy mode.'); throw new Error('You cannot use the storageService in histroy mode.');
} }

View File

@ -109,12 +109,13 @@ export class MotionBlockRepositoryService extends BaseAgendaContentObjectReposit
} }
/** /**
* Retrieves motion block(s) by name * Retrieves motion blocks by name
* TODO: check if a title is unique for a motionBlock *
* @param title Strign to check for * @param title String to check for
* @returns the motion blocks found
*/ */
public getMotionBlockByTitle(title: string): MotionBlock { public getMotionBlocksByTitle(title: string): MotionBlock[] {
return this.DS.find(MotionBlock, block => block.title === title); return this.DS.filter(MotionBlock, block => block.title === title);
} }
/** /**

View File

@ -265,14 +265,18 @@ export class UserRepositoryService extends BaseRepository<ViewUser, User> {
} }
/** /**
* Tries to convert a user string into an user. If it is two words, expect * Tries to convert a user string into an user. Names that don't fit the scheme given
* a first and a last name, if one word only, expect a first name only. * will be entered into the first_name field
* If more than two words, they will all be put as the first name *
* TODO: More advanced logic to fit names * Naming schemes are:
* - firstSpaceLast: One or two space-separated words are assumed, matching
* given name and surname
* - lastCommaFirst: A comma is supposed to separate last name(s) from given name(s).
* TODO: More advanced logic(s) to fit names
* *
* @param inputUser A raw user string * @param inputUser A raw user string
* @param schema optional hint on how to handle the strings. TODO: Not fully implemented. * @param schema optional hint on how to handle the strings.
* @returns A User object (not uploaded to the server) * @returns A User object (note: is only a local object, not uploaded to the server)
*/ */
public parseUserString(inputUser: string, schema?: StringNamingSchema): User { public parseUserString(inputUser: string, schema?: StringNamingSchema): User {
const newUser: Partial<User> = {}; const newUser: Partial<User> = {};

View File

@ -1,10 +1,10 @@
import { auditTime } from 'rxjs/operators'; import { auditTime } from 'rxjs/operators';
import { BehaviorSubject, Observable } from 'rxjs'; import { BehaviorSubject, Observable } from 'rxjs';
import { BaseRepository } from 'app/core/repositories/base-repository';
import { BaseModel } from '../../shared/models/base/base-model'; import { BaseModel } from '../../shared/models/base/base-model';
import { BaseViewModel } from '../../site/base/base-view-model'; import { BaseViewModel } from '../../site/base/base-view-model';
import { StorageService } from '../core-services/storage.service'; import { StorageService } from '../core-services/storage.service';
import { BaseRepository } from '../repositories/base-repository';
/** /**
* Describes the available filters for a listView. * Describes the available filters for a listView.
@ -46,7 +46,7 @@ export interface OsFilterOption {
* and will receive their filtered data as observable * and will receive their filtered data as observable
*/ */
export abstract class BaseFilterListService<M extends BaseModel, V extends BaseViewModel> { export abstract class BaseFilterListService<V extends BaseViewModel> {
/** /**
* stores the currently used raw data to be used for the filter * stores the currently used raw data to be used for the filter
*/ */
@ -110,7 +110,7 @@ export abstract class BaseFilterListService<M extends BaseModel, V extends BaseV
/** /**
* Constructor. * Constructor.
*/ */
public constructor(private store: StorageService, private repo: BaseRepository<V, M>) {} public constructor(private store: StorageService, private repo: BaseRepository<V, BaseModel>) {}
/** /**
* Initializes the filterService. Returns the filtered data as Observable * Initializes the filterService. Returns the filtered data as Observable

View File

@ -8,13 +8,16 @@ import { BaseViewModel } from 'app/site/base/base-view-model';
/** /**
* Interface for value- Label combinations. * Interface for value- Label combinations.
* Map objects didn't work, TODO: Use map objects (needs iterating through all objects of a map)
*/ */
export interface ValueLabelCombination { export interface ValueLabelCombination {
value: string; value: string;
label: string; label: string;
} }
interface FileReaderProgressEvent extends ProgressEvent {
readonly target: FileReader | null;
}
/** /**
* Interface matching a newly created entry with their duplicates and an import status * Interface matching a newly created entry with their duplicates and an import status
*/ */
@ -168,10 +171,8 @@ export abstract class BaseImportService<V extends BaseViewModel> {
* @param matSnackBar snackBar to display import errors * @param matSnackBar snackBar to display import errors
*/ */
public constructor(protected translate: TranslateService, private papa: Papa, protected matSnackbar: MatSnackBar) { public constructor(protected translate: TranslateService, private papa: Papa, protected matSnackbar: MatSnackBar) {
this.reader.onload = (event: any) => { this.reader.onload = (event: FileReaderProgressEvent) => {
// TODO type: event is a progressEvent, this.parseInput(event.target.result as string);
// but has a property target.result, which typescript doesn't recognize
this.parseInput(event.target.result);
}; };
} }

View File

@ -1,6 +1,7 @@
import { Output, Component, OnInit, EventEmitter, Input } from '@angular/core'; import { Output, Component, OnInit, EventEmitter, Input } from '@angular/core';
import { BaseFilterListService, OsFilterOption } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilterOption } from 'app/core/ui-services/base-filter-list.service';
import { BaseViewModel } from 'app/site/base/base-view-model';
/** /**
* Component for selecting the filters in a filter menu. * Component for selecting the filters in a filter menu.
@ -31,7 +32,7 @@ export class FilterMenuComponent implements OnInit {
* the FilterListService; unsure about how to get them in any other way. * the FilterListService; unsure about how to get them in any other way.
*/ */
@Input() @Input()
public service: BaseFilterListService<any, any>; // TODO (M, V) public service: BaseFilterListService<BaseViewModel>;
/** /**
* Constructor. Does nothing. * Constructor. Does nothing.

View File

@ -9,6 +9,7 @@ import { FilterMenuComponent } from './filter-menu/filter-menu.component';
import { OsSortingItem } from 'app/core/ui-services/base-sort-list.service'; import { OsSortingItem } from 'app/core/ui-services/base-sort-list.service';
import { BaseSortListService } from 'app/core/ui-services/base-sort-list.service'; import { BaseSortListService } from 'app/core/ui-services/base-sort-list.service';
import { ViewportService } from 'app/core/ui-services/viewport.service'; import { ViewportService } from 'app/core/ui-services/viewport.service';
import { BaseFilterListService } from 'app/core/ui-services/base-filter-list.service';
/** /**
* Reusable bar for list views, offering sorting and filter options. * Reusable bar for list views, offering sorting and filter options.
@ -47,7 +48,7 @@ export class OsSortFilterBarComponent<V extends BaseViewModel> {
* be a FilterListService extendingFilterListService. * be a FilterListService extendingFilterListService.
*/ */
@Input() @Input()
public filterService: any; // TODO a FilterListService extending FilterListService public filterService: BaseFilterListService<V>;
/** /**
* optional additional string to show after the item count. This string will not be translated here * optional additional string to show after the item count. This string will not be translated here
@ -80,7 +81,7 @@ export class OsSortFilterBarComponent<V extends BaseViewModel> {
*/ */
public get displayedCount(): number { public get displayedCount(): number {
if (this.filterCount === undefined || this.filterCount === null) { if (this.filterCount === undefined || this.filterCount === null) {
return this.filterService.filterCount; return this.filterService.filteredCount;
} else { } else {
return this.filterCount; return this.filterCount;
} }

View File

@ -205,7 +205,8 @@ export class AgendaImportService extends BaseImportService<ViewCreateTopic> {
}; };
const duplicates = this.repo.getTopicDuplicates(newTopic); const duplicates = this.repo.getTopicDuplicates(newTopic);
if (duplicates.length) { if (duplicates.length) {
// TODO this is a dishonest casting. duplicates should not be required to be View // TODO duplicates are not really ViewCreateTopics, but ViewTopics.
// TODO this should be fine as the duplicates will not be created
newEntry.duplicates = duplicates as ViewCreateTopic[]; newEntry.duplicates = duplicates as ViewCreateTopic[];
this.setError(newEntry, 'Duplicates'); this.setError(newEntry, 'Duplicates');
} }

View File

@ -1,7 +1,7 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { BaseFilterListService, OsFilter, OsFilterOption } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilter, OsFilterOption } from 'app/core/ui-services/base-filter-list.service';
import { Item, itemVisibilityChoices } from 'app/shared/models/agenda/item'; import { itemVisibilityChoices } from 'app/shared/models/agenda/item';
import { ViewItem } from '../models/view-item'; import { ViewItem } from '../models/view-item';
import { StorageService } from 'app/core/core-services/storage.service'; import { StorageService } from 'app/core/core-services/storage.service';
import { ItemRepositoryService } from 'app/core/repositories/agenda/item-repository.service'; import { ItemRepositoryService } from 'app/core/repositories/agenda/item-repository.service';
@ -10,7 +10,7 @@ import { TranslateService } from '@ngx-translate/core';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
}) })
export class AgendaFilterListService extends BaseFilterListService<Item, ViewItem> { export class AgendaFilterListService extends BaseFilterListService<ViewItem> {
protected name = 'Agenda'; protected name = 'Agenda';
public filterOptions: OsFilter[] = []; public filterOptions: OsFilter[] = [];

View File

@ -65,7 +65,7 @@ export class AgendaPdfService {
/** /**
* Parses an entry line and triggers parsing of any children * Parses an entry line and triggers parsing of any children
* (TODO: Check assumption: items with 'is_hidden' are not to be exported) * Items with 'is_hidden' and their subitems are not exported
* *
* @param nodeItem the item for the head line * @param nodeItem the item for the head line
* @param level: The hierarchy index (beginning at 0 for top level agenda topics) * @param level: The hierarchy index (beginning at 0 for top level agenda topics)

View File

@ -67,7 +67,7 @@ export class AssignmentListComponent extends ListViewBaseComponent<ViewAssignmen
* Sets the title, inits the table * Sets the title, inits the table
*/ */
public ngOnInit(): void { public ngOnInit(): void {
super.setTitle(this.translate.instant('Elections')); super.setTitle('Elections');
this.initTable(); this.initTable();
} }

View File

@ -1,7 +1,6 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { AssignmentRepositoryService } from 'app/core/repositories/assignments/assignment-repository.service'; import { AssignmentRepositoryService } from 'app/core/repositories/assignments/assignment-repository.service';
import { Assignment } from 'app/shared/models/assignments/assignment';
import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service';
import { StorageService } from 'app/core/core-services/storage.service'; import { StorageService } from 'app/core/core-services/storage.service';
import { ViewAssignment, AssignmentPhases } from '../models/view-assignment'; import { ViewAssignment, AssignmentPhases } from '../models/view-assignment';
@ -9,7 +8,7 @@ import { ViewAssignment, AssignmentPhases } from '../models/view-assignment';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
}) })
export class AssignmentFilterListService extends BaseFilterListService<Assignment, ViewAssignment> { export class AssignmentFilterListService extends BaseFilterListService<ViewAssignment> {
protected name = 'Assignment'; protected name = 'Assignment';
/** /**

View File

@ -89,7 +89,7 @@ export abstract class ListViewBaseComponent<V extends BaseViewModel, M extends B
matSnackBar: MatSnackBar, matSnackBar: MatSnackBar,
protected route?: ActivatedRoute, protected route?: ActivatedRoute,
protected storage?: StorageService, protected storage?: StorageService,
public filterService?: BaseFilterListService<M, V>, public filterService?: BaseFilterListService<V>,
public sortService?: BaseSortListService<V> public sortService?: BaseSortListService<V>
) { ) {
super(titleService, translate, matSnackBar); super(titleService, translate, matSnackBar);

View File

@ -12,9 +12,13 @@ import { ViewConfig } from '../../models/view-config';
import { ConfigRepositoryService } from 'app/core/repositories/config/config-repository.service'; import { ConfigRepositoryService } from 'app/core/repositories/config/config-repository.service';
/** /**
* List view for the categories. * Component for a config field, used by the {@link ConfigListComponent}. Handles
* all inpu types defined by the server, as well as updating the configs
* *
* TODO: Creation of new Categories * @example
* ```ts
* <os-config-field [item]="item.config"></os-config-field>
* ```
*/ */
@Component({ @Component({
selector: 'os-config-field', selector: 'os-config-field',
@ -58,7 +62,8 @@ export class ConfigFieldComponent extends BaseComponent implements OnInit {
public rawDate: Date; public rawDate: Date;
/** /**
* The config item for this component. Just accept components with already populated constants-info. * The config item for this component. Just accepts components with already
* populated constants-info.
*/ */
@Input() @Input()
public set item(value: ViewConfig) { public set item(value: ViewConfig) {

View File

@ -1,7 +1,6 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service';
import { Mediafile } from 'app/shared/models/mediafiles/mediafile';
import { ViewMediafile } from '../models/view-mediafile'; import { ViewMediafile } from '../models/view-mediafile';
import { StorageService } from 'app/core/core-services/storage.service'; import { StorageService } from 'app/core/core-services/storage.service';
import { MediafileRepositoryService } from 'app/core/repositories/mediafiles/mediafile-repository.service'; import { MediafileRepositoryService } from 'app/core/repositories/mediafiles/mediafile-repository.service';
@ -11,7 +10,7 @@ import { TranslateService } from '@ngx-translate/core';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
}) })
export class MediafileFilterListService extends BaseFilterListService<Mediafile, ViewMediafile> { export class MediafileFilterListService extends BaseFilterListService<ViewMediafile> {
protected name = 'Mediafile'; protected name = 'Mediafile';
/** /**

View File

@ -3,7 +3,6 @@ import { Injectable } from '@angular/core';
import { TranslateService } from '@ngx-translate/core'; import { TranslateService } from '@ngx-translate/core';
import { BaseFilterListService, OsFilter, OsFilterOptions } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilter, OsFilterOptions } from 'app/core/ui-services/base-filter-list.service';
import { Motion } from 'app/shared/models/motions/motion';
import { ViewMotion } from '../models/view-motion'; import { ViewMotion } from '../models/view-motion';
import { CategoryRepositoryService } from 'app/core/repositories/motions/category-repository.service'; import { CategoryRepositoryService } from 'app/core/repositories/motions/category-repository.service';
import { WorkflowRepositoryService } from 'app/core/repositories/motions/workflow-repository.service'; import { WorkflowRepositoryService } from 'app/core/repositories/motions/workflow-repository.service';
@ -19,7 +18,7 @@ import { TagRepositoryService } from 'app/core/repositories/tags/tag-repository.
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
}) })
export class MotionFilterListService extends BaseFilterListService<Motion, ViewMotion> { export class MotionFilterListService extends BaseFilterListService<ViewMotion> {
protected name = 'Motion'; protected name = 'Motion';
/** /**

View File

@ -271,12 +271,12 @@ export class MotionImportService extends BaseImportService<ViewMotion> {
return null; return null;
} }
blockString = blockString.trim(); blockString = blockString.trim();
let existingBlock = this.motionBlockRepo.getMotionBlockByTitle(blockString); let existingBlock = this.motionBlockRepo.getMotionBlocksByTitle(blockString);
if (!existingBlock) { if (!existingBlock.length) {
existingBlock = this.motionBlockRepo.getMotionBlockByTitle(this.translate.instant(blockString)); existingBlock = this.motionBlockRepo.getMotionBlocksByTitle(this.translate.instant(blockString));
} }
if (existingBlock) { if (existingBlock.length) {
return { id: existingBlock.id, name: existingBlock.title }; return { id: existingBlock[0].id, name: existingBlock[0].title };
} else { } else {
if (!this.newMotionBlocks.find(newBlock => newBlock.name === blockString)) { if (!this.newMotionBlocks.find(newBlock => newBlock.name === blockString)) {
this.newMotionBlocks.push({ name: blockString }); this.newMotionBlocks.push({ name: blockString });

View File

@ -95,11 +95,10 @@ export class UserListComponent extends ListViewBaseComponent<ViewUser, User> imp
private _presenceViewConfigured = false; private _presenceViewConfigured = false;
/** /**
* TODO: Does not check for user manage rights itself
* @returns true if the presence view is available to administrators * @returns true if the presence view is available to administrators
*/ */
public get presenceViewConfigured(): boolean { public get presenceViewConfigured(): boolean {
return this._presenceViewConfigured; return this._presenceViewConfigured && this.operator.hasPerms('users.can_manage');
} }
/** /**
@ -164,7 +163,7 @@ export class UserListComponent extends ListViewBaseComponent<ViewUser, User> imp
* to filter/sort services * to filter/sort services
*/ */
public ngOnInit(): void { public ngOnInit(): void {
super.setTitle(this.translate.instant('Participants')); super.setTitle('Participants');
this.initTable(); this.initTable();
this.setFulltextFilter(); this.setFulltextFilter();

View File

@ -97,7 +97,6 @@ export class ViewUser extends BaseProjectableViewModel implements Searchable {
return this.user && !!this.user.last_email_send; return this.user && !!this.user.last_email_send;
} }
// TODO read config values for "users_sort_by"
/** /**
* Getter for the short name (Title, given name, surname) * Getter for the short name (Title, given name, surname)
* *
@ -111,16 +110,6 @@ export class ViewUser extends BaseProjectableViewModel implements Searchable {
const title = this.title ? this.title.trim() : ''; const title = this.title ? this.title.trim() : '';
const firstName = this.first_name ? this.first_name.trim() : ''; const firstName = this.first_name ? this.first_name.trim() : '';
const lastName = this.last_name ? this.last_name.trim() : ''; const lastName = this.last_name ? this.last_name.trim() : '';
// TODO need DS adjustment first first
// if (this.DS.getConfig('users_sort_by').value === 'last_name') {
// if (lastName && firstName) {
// shortName += `${lastName}, ${firstName}`;
// } else {
// shortName += lastName || firstName;
// }
// }
let shortName = `${firstName} ${lastName}`; let shortName = `${firstName} ${lastName}`;
if (shortName.length <= 1) { if (shortName.length <= 1) {

View File

@ -2,7 +2,6 @@ import { Injectable } from '@angular/core';
import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service'; import { BaseFilterListService, OsFilter } from 'app/core/ui-services/base-filter-list.service';
import { StorageService } from 'app/core/core-services/storage.service'; import { StorageService } from 'app/core/core-services/storage.service';
import { User } from 'app/shared/models/users/user';
import { ViewUser } from '../models/view-user'; import { ViewUser } from '../models/view-user';
import { GroupRepositoryService } from 'app/core/repositories/users/group-repository.service'; import { GroupRepositoryService } from 'app/core/repositories/users/group-repository.service';
import { UserRepositoryService } from 'app/core/repositories/users/user-repository.service'; import { UserRepositoryService } from 'app/core/repositories/users/user-repository.service';
@ -11,7 +10,7 @@ import { TranslateService } from '@ngx-translate/core';
@Injectable({ @Injectable({
providedIn: 'root' providedIn: 'root'
}) })
export class UserFilterListService extends BaseFilterListService<User, ViewUser> { export class UserFilterListService extends BaseFilterListService<ViewUser> {
protected name = 'User'; protected name = 'User';
private userGroupFilterOptions = { private userGroupFilterOptions = {

View File

@ -22,7 +22,6 @@ export class UserSortListService extends BaseSortListService<ViewUser> {
{ property: 'number', label: 'Participant number' }, { property: 'number', label: 'Participant number' },
{ property: 'structure_level', label: 'Structure level' }, { property: 'structure_level', label: 'Structure level' },
{ property: 'comment' } { property: 'comment' }
// TODO email send?
] ]
}; };
protected name = 'User'; protected name = 'User';