Skip to content
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

WIP: Fix Delete Slide #990

Merged
merged 7 commits into from
Aug 26, 2024
Merged

WIP: Fix Delete Slide #990

merged 7 commits into from
Aug 26, 2024

Conversation

arifpriadi
Copy link
Contributor

Issue #989

@pandigresik
Copy link
Contributor

Untuk nama file hasil upload, usahakan tidak menggunakan nama file yang diupload $fileName = $file->getClientOriginalName()
referensi : https://laravel.com/docs/10.x/filesystem#other-uploaded-file-information

bisa juga menggunakan uuid untuk nama file gambar yang disimpan

@vickyrolanda
Copy link
Contributor

bung @arifpriadi mohon untuk pembaharuannya agar tim OpenSID bisa masukkan PR ini ke Rilis.

@arifpriadi
Copy link
Contributor Author

Untuk nama file hasil upload, usahakan tidak menggunakan nama file yang diupload $fileName = $file->getClientOriginalName() referensi : https://laravel.com/docs/10.x/filesystem#other-uploaded-file-information

bisa juga menggunakan uuid untuk nama file gambar yang disimpan

Pak @pandigresik, pada PR ini hanya memperbaiki notifikasi yang tidak sesuai disebabkan karena file yang akan dihapus tidak ditemukan. Tidak ada kaitannya dengan unggah. Jika memasang dengan data demo, maka pada terdapat beberapa slider. Bisa dicoba pada data tsb.

@pandigresik
Copy link
Contributor

Untuk nama file hasil upload, usahakan tidak menggunakan nama file yang diupload $fileName = $file->getClientOriginalName() referensi : https://laravel.com/docs/10.x/filesystem#other-uploaded-file-information
bisa juga menggunakan uuid untuk nama file gambar yang disimpan

Pak @pandigresik, pada PR ini hanya memperbaiki notifikasi yang tidak sesuai disebabkan karena file yang akan dihapus tidak ditemukan. Tidak ada kaitannya dengan unggah. Jika memasang dengan data demo, maka pada terdapat beberapa slider. Bisa dicoba pada data tsb.

Ketika lakukan perbaikan lebih baik sekalian, pada fitur delete digunakan penghapusan melalui model. Maka alangkah baiknya ubah juga fitur create/update menggunakan metode yang sama agar seragam yaitu penghapusan melalui model. Pada fitur slide ketika unggah file masih menggunakan nama file asli dari unggahan, akan lebih baik jika gunakan nama file random yang sulit ditebak dengan alasan keamanan. Sekalian diperbaiki karena ubahan di model juga

@arifpriadi
Copy link
Contributor Author

Untuk nama file hasil upload, usahakan tidak menggunakan nama file yang diupload $fileName = $file->getClientOriginalName() referensi : https://laravel.com/docs/10.x/filesystem#other-uploaded-file-information
bisa juga menggunakan uuid untuk nama file gambar yang disimpan

Pak @pandigresik, pada PR ini hanya memperbaiki notifikasi yang tidak sesuai disebabkan karena file yang akan dihapus tidak ditemukan. Tidak ada kaitannya dengan unggah. Jika memasang dengan data demo, maka pada terdapat beberapa slider. Bisa dicoba pada data tsb.

Ketika lakukan perbaikan lebih baik sekalian, pada fitur delete digunakan penghapusan melalui model. Maka alangkah baiknya ubah juga fitur create/update menggunakan metode yang sama agar seragam yaitu penghapusan melalui model. Pada fitur slide ketika unggah file masih menggunakan nama file asli dari unggahan, akan lebih baik jika gunakan nama file random yang sulit ditebak dengan alasan keamanan. Sekalian diperbaiki karena ubahan di model juga

Disesuaikan dengan saran yang diberikan. Mencontoh dari https://github.com/OpenSID/premium/blob/rilis-dev/app/Models/SinergiProgram.php#L112-L134
image

@pandigresik pandigresik merged commit c1c7de9 into OpenSID:dev Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants