-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add minio storage #121
base: v2
Are you sure you want to change the base?
Add minio storage #121
Conversation
145aed3
to
d7683ef
Compare
|
||
if ($this->createBucket($this->publicBucketName, $style)) { | ||
//Add DL rights for public bucket | ||
$policyReadOnly = '{ |
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.
injecter la policy depuis le services.yml (en tant que parameter)
|
||
use Symfony\Component\HttpFoundation\File\UploadedFile; | ||
|
||
trait ProfilePicture |
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.
can we use a validation constraint on file? I think https://symfony.com/doc/current/reference/constraints/Image.html should be good. We may need to add a max file size value aswell
Additionnaly, We should suffix trait with Trait
(we should probably prefer class composition over traits)
|
||
use Symfony\Component\Validator\Constraints as Assert; | ||
|
||
class CreateUserDto | ||
{ | ||
use ProfilePicture; |
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.
je comprend pas l'intérêt du trait alors que c'est l'unique endroit où il est utilisé. Peut être créer un trait plus générique (du style, ImageTrait) mais je ne suis pas sûr de l'utilité
return $this->name; | ||
} | ||
|
||
public function setName(string $name): static |
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.
I personally dont like "fluent interface" or "return this" in most cases, I think its less readable in most cases (more than one execution per line + you can't easily know what is going on, for example $user->setName("foo")->calculateTime()->getCompany()->setName()
, or worst $company->setUser($user->setName("foo"))
). (weirdly the doctrine creator seems to agree with me )
We should decide on a general guideline about that
|
||
export default async function useGetCompany(companyId: string) { | ||
return useAppFetch<Company>(() => "/companies/" + companyId, { | ||
key: "getCompany", |
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.
remove the key usage or use the companyId in the key
import { GET } from "~/constants/http"; | ||
import useAppFetch from "~/composables/useAppFetch"; | ||
|
||
export default async function useListUsers() { |
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.
this is suspicious (wrong name)
|
||
const route = useRoute(); | ||
|
||
const { data: company, pending: companyPending } = await useGetCompany( |
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.
you should use the {{ error }}
@click="onDeleteCompany(company)" | ||
> | ||
Delete | ||
</button> |
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.
use prime components?
|
||
const onDeleteCompany = async (company: Company) => { | ||
try { | ||
await deleteCompany(company); |
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.
error should be handled
No description provided.