Conversation
BlueTechie
force-pushed
the
rds/upload-custom-templates
branch
from
c46adb6 to
c7cf20f
Compare
ValentinFutterer
left a comment
ValentinFutterer
left a comment
There was a problem hiding this comment.
Almost just small things in the comments, but exporting needs a little bit more testing.
| .current-grid { | ||
| display: grid; | ||
| grid-template-columns: repeat(3, 1fr); | ||
| gap: 10px; | ||
| padding: 0 8px 8px 8px; | ||
| } | ||
|
|
||
| .current-grid .centered { | ||
| grid-column: 1 / span 3; | ||
| margin: 0 auto; | ||
| justify-self: center; | ||
| } |
There was a problem hiding this comment.
3 grid columns work fine mostly, but it leaves a lot of empty space here
Its not backbreaking but maybe there is an easy solution to make it dynamic. Otherwise its also fine as is.
| if (!isExtensionValid && !isMimeValid) { | ||
| this.feedbackService.error( | ||
| 'Only Microsoft Word files (.docx or .doc) are allowed.', |
There was a problem hiding this comment.
This text is hardcoded and not necessary. There is a second error that pops up and says something similar.
| const config$ = this.http.get<Config>(`${host}config`); | ||
| const rawConfig = await lastValueFrom(config$); | ||
| this.config = rawConfig; | ||
| this.configSubject.next(rawConfig); |
There was a problem hiding this comment.
You can remove this line and all mentions of the subject from the file - its never queried only written to.
| next: () => { | ||
| this.configService.refreshConfig().then(() => { | ||
| this.feedbackService.success('admin.templates.success.status'); | ||
| }); | ||
| }, | ||
| error: () => { | ||
| template.active = previousState; |
There was a problem hiding this comment.
Just FYI, this doesnt take into account the config call failing, so the frontend just continues as is. Maybe add error handling to the refresh method?
| "admin.template": "Templates", | ||
| "admin.edit.templates": "Edit templates", |
There was a problem hiding this comment.
| "admin.template": "Templates", | |
| "admin.edit.templates": "Edit templates", | |
| "admin.template.sectionTitle": "Templates", | |
| "admin.edit.templates": "Edit templates", |
check the conventions on naming
| } | ||
|
|
||
| private isValidWordFile(file: File): boolean { | ||
| const allowedExtensions = ['.docx', '.doc']; |
There was a problem hiding this comment.
The file upload component itself only allows .docx
| const previousTemplates = this.localTemplates(); | ||
|
|
||
| this.localTemplates.update(templates => templates.filter(t => t.id !== id)); | ||
|
|
||
| this.backendService.deleteExportTemplate(id).subscribe({ | ||
| next: () => { | ||
| this.configService.refreshConfig().then(() => { | ||
| this.feedbackService.success('admin.templates.success.delete'); | ||
| }); | ||
| }, | ||
| error: err => { | ||
| this.localTemplates.set(previousTemplates); |
There was a problem hiding this comment.
No ned to store previous templates, you could just pdate local templates in the success case.
| }); | ||
| } | ||
|
|
||
| private isValidWordFile(file: File): boolean { |
There was a problem hiding this comment.
There should also be a basic size check. Max size can be generous.
| if (result && result !== 'cancel') { | ||
| const template = result; | ||
| if (!funderSupported) { | ||
| this.exportDmpType = template; |
There was a problem hiding this comment.
Normal export fails when you deactivate the template type the project has. For example deactivate HE template and then try to export a HE project DMP. Testexporting again using preview and export in different scenarios.
| readonly activeTemplates = computed(() => { | ||
| const config = this.configService.getConfig(); | ||
| // Filter for active templates only | ||
| return config?.templates?.filter(t => t.active) || []; | ||
| }); |
There was a problem hiding this comment.
This method is duplicated, maybe put it inside of configservice or config?
2 participants
Description
Feature
What does this PR do?
Backend PR description for context: damap-org/damap-backend#471
In short: this frontend PR adds an
admin/templatespage where admins can add custom variations of the Science Europe template. They can also delete these custom templates and toggle the active status of all templates (the 3 base templates as well as the custom ones).EditTemplatesPageComponentfor managing export templates..docxextension enforcement (the backend also does this but in a more proper way using theFitsservice).en.jsonand in the new flattranslations.jsonfile.Dependencies
Backend PRs: damap-org/damap-backend#471
closes GH-