Conversation
Lars Kemper (larskemper)
requested review from
a team,
Simon Fiebranz (CR0YD),
Dennis Garding (DennisGarding),
Malte Janz (MalteJanz),
Susanne Hartung (ennasus4sun),
Jozsef Damokos (jozsefdamokos) and
Nikolas Evers (vintagesucks)
and removed request for
a team
| name: 'api.admin.migration.download-logs-of-run', | ||
| methods: [Request::METHOD_POST], | ||
| defaults: ['auth_required' => false, PlatformRequest::ATTRIBUTE_ACL => ['swag_migration.viewer']] | ||
| defaults: [PlatformRequest::ATTRIBUTE_ACL => ['swag_migration.viewer']] |
There was a problem hiding this comment.
Have you manually tested and confirmed that the download still works fine? I could imagine that the auth_required flag was there on purpose to support the way JS was calling it 🤔
| path: '/api/_action/migration/check-connection', | ||
| name: 'api.admin.migration.check-connection', | ||
| defaults: [PlatformRequest::ATTRIBUTE_ACL => ['swag_migration.viewer']], | ||
| defaults: [PlatformRequest::ATTRIBUTE_ACL => ['swag_migration.editor']], |
There was a problem hiding this comment.
Is editor really the right call here? Because I think every time you open the migration module we perform a connection check and if you are only viewer, what else is left that you could view?
| this.$refs.downloadForm.submit(); | ||
| async submitDownload() { | ||
| try { | ||
| const blob = await this.migrationApiService.downloadLogsOfRun(this.migrationRun.id); |
There was a problem hiding this comment.
Not sure if this is best practice, would need to research that myself. I could imagine that this could be annoying for larger files (which the log files could be). Imagine you clicking on download and nothing happens for 20s because you have a slow connection and its a bigger file. Then suddenly you get your browser download showing up and finishing instantly.
I'm not sure how this behaves but it's worth a closer look. I think the old implementation let the browser do the download from an URL, which would show proper progress in the download UI of the browser 🤔
3 participants
related https://github.com/shopware/shopware-private/issues/217
Changes
/api/_action/migration/check-connectionshould haveswag_migration.editoras acl role (finding)/api/_action/migration/download-logs-of-runshould require authenticationauth_required = true(finding)