XML agenda export - #6461
Conversation
| itemMap: Map<number, ViewAgendaItem<any>>, | ||
| isNested = false | ||
| ): HTMLElement { | ||
| const agendaItem = doc.createElement(isNested ? 'Sub-Agenda-Item' : 'Agenda-Item'); |
There was a problem hiding this comment.
I dislike the upper camel case and kebab case together. (and sometimes also snake_case)
Change it to kebab case only
Also apply to everything below too
There was a problem hiding this comment.
Please state why the string still has upper camel case and kebab case.
| metaInfo: [`duration`] | ||
| }; | ||
|
|
||
| private xmlDefaults = this.csvDefaults; |
There was a problem hiding this comment.
either
A) copy line 115 and 116 directly and fill xmlDefault like that
B) unify csv and XML default, so it's usable at the same time as the same variable
C) create a new variable to reference the content and lett all default call this new variable
|
|
||
| export type csvMetaInfo = `duration` | `tags` | `agenda_visibility` | `done`; | ||
|
|
||
| export type xmlMetaInfo = csvMetaInfo; |
There was a problem hiding this comment.
make the content explicit or rename to represent both (CSV and XML)
| const addContentNode = (name: InfoToExport | xmlMetaInfo, value: unknown): void => { | ||
| if (config.includes(name) && this.isEmpty(value)) { | ||
| const node = doc.createElement(name.replaceAll('_', '-')); | ||
| node.textContent = String(value); | ||
| agendaItem.appendChild(node); | ||
| } | ||
| }; |
There was a problem hiding this comment.
- this needs to be it's own function
- we do not use
: unknown. use: anyif nothing other is possible
for here use: string | number | boolean - check the type of value. if value is a string just fill node.textContent, if value is no string use
.toString()
| itemMap: Map<number, ViewAgendaItem<any>>, | ||
| isNested = false | ||
| ): HTMLElement { | ||
| const agendaItem = doc.createElement(isNested ? 'Sub-Agenda-Item' : 'Agenda-Item'); |
There was a problem hiding this comment.
Please state why the string still has upper camel case and kebab case.
| if (item.tags?.length && config.includes('tags')) { | ||
| const tags = doc.createElement('tags'); | ||
| item.tags.filter(tagName => { | ||
| const tag = doc.createElement('tag'); | ||
| tag.textContent = tagName.tag.name; | ||
| tags.appendChild(tag); | ||
| }); | ||
| agendaItem.appendChild(tags); |
There was a problem hiding this comment.
use your own function addContentNode
|
|
||
| public exportAsXML(source: ViewAgendaItem[], info: InfoToExport[], xmlMeta: xmlMetaInfo[]): void { | ||
| const filename = this.translate.instant(`Agenda`) + `.xml`; | ||
| const config: (InfoToExport | csvMetaInfo)[] = [...info, ...xmlMeta]; |
|
|
||
| import { ViewAgendaItem } from '../../pages/agenda'; | ||
| import { | ||
| csvMetaInfo, |
There was a problem hiding this comment.
I'll say it here once:
using csvMetaInfo is wrong.
either fix the naming convention (see comment above) or exchange the csv with xml.
you won't be needing the csvMetaInfo in the xml-export
| export class MeetingXmlExportService { | ||
| private exporter = inject(FileExportService); | ||
| private serializer = new XMLSerializer(); | ||
| private itemMap; |
There was a problem hiding this comment.
| private itemMap; | |
| private itemMap: Map<number, ViewAgendaItem<any>>; |
| return agendaItem; | ||
| } | ||
|
|
||
| private isEmpty(value: any): boolean { |
There was a problem hiding this comment.
this function returns true if the value is NOT empty
Resolves half of #6323