-
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
File Uploads #149
base: development
Are you sure you want to change the base?
The head ref may contain hidden characters: "feature/123-anh\u00E4nge-f\u00FCr-die-tn-bei-der-anmeldung"
File Uploads #149
Conversation
make file input more beatiful
add environment vars for config add helm chart add azure folder support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier gibt's noch ein paar Optimierungspotenziale in meinen Augen.
documents UnterveranstaltungDocument[] | ||
} | ||
|
||
model UnterveranstaltungDocument { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wozu diese extra Entität? Verknüpfung Unterveranstaltung -> File reicht in meinen Augen, dann als Array einfach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der Gedanke hier ist, dass Dokumente auch einen beschreibenden Namen enthalten kann. Theoretisch könnte man das auch in File packen.
uploaded Boolean @default(false) | ||
uploadedAt DateTime? | ||
provider FileProvider | ||
key String @unique() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ID als Unique/Primary reicht denke ich
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der key hat hier eine andere Funktion als ID. Für den azure provider wird dort der key/pfad abgelegt, der dann zur azure-resource führt. Ich wollte vermeiden, dass die id Zeichen wie /
enthählt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah und die ID hab ich zu einem UUID gemacht, damit die nicht zu erraten ist
|
||
export default function addMiddlewares(router: Router) { | ||
router.post('/upload/anmeldungen', async (ctx, next) => { | ||
return await importAnmeldungen(ctx, next) | ||
}) | ||
router.post('/upload/file/LOCAL/:id', uploadFileLocal) | ||
router.get('/download/file/LOCAL/:id', downloadFileLocal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der Provider lässt sich sicher auch parametrisieren
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hab ich weggelassen, da der endpunkt nur für den LOCAL provider genutzt werden soll. Bei Azure wird direkt bei Azure die datei angefragt.
Wenn man weitere provider-arten hinzufügt sollte, würde ich hier weitere endpunkte anlegen, damit das Isoliert bleibt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
export const downloadFileLocal: Middleware = async function (ctx, next) { | ||
// TODO: add authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier ist noch eine ToDo
// const { state: fileList } = useAsyncState(async () => { | ||
// // return apiClient. | ||
// apiClient.file.List.query({ filter: {}, pagination: { take: 100, skip: 0 } }) | ||
// // return apiClient.gliederung.verwaltungList.query({ filter: {}, pagination: { take: 100, skip: 0 } }) | ||
// }, []) | ||
|
||
// await fetch('http://localhost:3030/upload/file/1', { | ||
// method: 'POST', | ||
// body: 'foo', | ||
// }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier ist noch ein Kommentar
In der Ausschreibung einer Unterveranstaltung können nun Dateien hochgeladen werden, die in der Öffentlichen Ausschreibungsseite gelistet und heruntergeladen werden können.
Dazu habe ich ein Datei-Upload und -Download umgesetzt. Das habe ich in zwei Provider unterteilt,
LOCAL
undAZURE
.Provider LOCAL
Da dieses Repo öffentlich ist und nicht alle Entwickler ein azure-zugang haben, gibt es den
LOCAL
-Provider. Hier werden die Dateien in ein lokalen Ordner gespeichert. Im devcontainer ist das im projekt-ordner der Ordneruploads
. Dieser steht auch im gitignore.Es sollten keine weiteren Schritte für Entwickler nötig sein, damit das funktioniert.
Provider AZURE
Für Azure werden die credentials
account
,accountKey
,container
undfolder
benötigt. Diese können einfach in die helm-values übergeben werden und es sollte alles funktionieren. Lokal habe ich das erfolgreich über Umgebungsvariablen getestet.DEVOPS NOTES
Als default in den helm-charts wird der provider
LOCAL
verwendet mit dem Pfad/tmp
. Dieser Ordner ist nur temporär und wird bei pod-neustarts geleert. Daher müssen die azure-credentials hinterlegt werden, damit alles auch nach einem pod-neustart funktioniert.Vielleicht kann @superbarne die helm-files reviewen
closes #123
Eventuell kann der Datei-Upload auch für #49 #48 interessant sein.