Miroslavsckaya/copypaste
1
0
Fork 0
Code Issues 1 Pull requests 1 Packages Projects Releases Wiki Activity

WIP: feature_paste #1

Draft
Miroslavsckaya wants to merge 24 commits from feature_paste into master
pull from: feature_paste
merge into: Miroslavsckaya:master
Conversation 0 Commits 24 Files changed 31 +8 -9
5 changed files with 8 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit ea7ddcde65 - Show all commits

2
migrations/Version20230720115905.php
View file

@ -13,7 +13,7 @@ final class Version20230720115905 extends AbstractMigration
public function up(Schema $schema): void public function up(Schema $schema): void
skobkin marked this conversation as resolved
skobkin commented 2023-07-21 22:27:19 +00:00
Review
Copy link

Do you need it here if it's empty?

Do you need it here if it's empty?
👍 1
{ {
$this->addSql('CREATE SEQUENCE paste_id_seq INCREMENT BY 1 MINVALUE 1 START 1'); $this->addSql('CREATE SEQUENCE paste_id_seq INCREMENT BY 1 MINVALUE 1 START 1');
$this->addSql('CREATE TABLE paste (id INT NOT NULL, text TEXT NOT NULL, language VARCHAR(25), description TEXT, filename VARCHAR(128), author VARCHAR(128) NOT NULL, publish_date TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, expiration_date TIMESTAMP(0) WITHOUT TIME ZONE, ip VARCHAR(15) NOT NULL, secret VARCHAR(40), PRIMARY KEY(id))'); $this->addSql('CREATE TABLE paste (id INT NOT NULL, text TEXT NOT NULL, language VARCHAR(25), description TEXT, filename VARCHAR(128), author VARCHAR(128), publish_date TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL, expiration_date TIMESTAMP(0) WITHOUT TIME ZONE, ip VARCHAR(15) NOT NULL, secret VARCHAR(40), PRIMARY KEY(id))');
} }
public function down(Schema $schema): void public function down(Schema $schema): void

2
src/Controller/PasteController.php
View file

@ -37,7 +37,7 @@ class PasteController extends AbstractController
} }
#[Route('/{id}/{secret}')] #[Route('/{id}/{secret}')]
skobkin marked this conversation as resolved Outdated
skobkin commented 2023-07-21 22:24:36 +00:00
Outdated
Review
Copy link

I mean you don't need to pass creation date here at all.

I mean you don't need to pass creation date here at all.
👍 1
public function showPaste(PasteRepository $pasteRepository, string $id, ?string $secret=NULL): Response public function showPaste(PasteRepository $pasteRepository, string $id, ?string $secret = NULL): Response
skobkin commented 2023-08-06 11:03:34 +00:00
Review
Copy link

show_paste at least.

`show_paste` at least.
{ {
$paste = $pasteRepository->findOneBy(['id' => $id, 'secret' => $secret]); $paste = $pasteRepository->findOneBy(['id' => $id, 'secret' => $secret]);
skobkin marked this conversation as resolved Outdated
skobkin commented 2023-07-21 22:40:57 +00:00
Outdated
Review
Copy link
  • it's a good idea to add , to the last argument too if you use multi-line formatting
  • if you're doing this in the controller, you can just do something like $pasteData->private ? \hash('sha1', \random_bytes(25)) : null. But I suggest to do that in the static method (named constructor) as I also suggested nearby.
- it's a good idea to add `,` to the last argument too if you use multi-line formatting - if you're doing this in the controller, you can just do something like `$pasteData->private ? \hash('sha1', \random_bytes(25)) : null`. But I suggest to do that in the static method (named constructor) as I also suggested nearby.
👍 1
$pasteData = new PasteFormData($paste); $pasteData = new PasteFormData($paste);

7
src/DTO/PasteFormData.php
View file

@ -15,11 +15,10 @@ class PasteFormData
public ?string $language = null; public ?string $language = null;
public ?string $description = null; public ?string $description = null;
public ?string $filename = null; public ?string $filename = null;
#[Assert\NotBlank] public ?string $author = null;
public string $author = 'anonymous';
public ?\DateTimeImmutable $expirationDate; public ?\DateTimeImmutable $expirationDate;
skobkin marked this conversation as resolved
skobkin commented 2023-07-21 22:36:37 +00:00
Review
Copy link

Why string and Assert\NotBlank?

Why `string` and `Assert\NotBlank`?
👍 1
public function __construct(?Paste $paste=null) public function __construct(?Paste $paste = null)
{ {
skobkin marked this conversation as resolved
skobkin commented 2023-07-22 18:00:09 +00:00
Review
Copy link

Is this validation being processed BEFORE storing the data in the DTO?
If not, it's pointless as with bool field earlier.

Is this validation being processed BEFORE storing the data in the DTO? If not, it's pointless as with `bool` field earlier.
👍 1
if ($paste === null) if ($paste === null)
{ {
skobkin marked this conversation as resolved
skobkin commented 2023-07-21 22:33:47 +00:00
Review
Copy link

Or you can make constructor private, use property promotion and add fromPaste() method to create it from the entity.

Or you can make constructor `private`, use property promotion and add `fromPaste()` method to create it from the entity.
👍 1
@ -30,7 +29,7 @@ class PasteFormData
$this->language = $paste->language; $this->language = $paste->language;
$this->description = $paste->description; $this->description = $paste->description;
$this->filename = $paste->filename; $this->filename = $paste->filename;
$this->author = $paste->author; $this->author = $paste->author !== null ? $paste->author : 'anonymous';
$this->expirationDate = $paste->expirationDate; $this->expirationDate = $paste->expirationDate;
} }
} }

4
src/Entity/Paste.php
View file

@ -23,8 +23,8 @@ class Paste
public readonly ?string $description, public readonly ?string $description,
#[ORM\Column(type: 'string', length: 128, nullable: true)] #[ORM\Column(type: 'string', length: 128, nullable: true)]
public readonly ?string $filename, public readonly ?string $filename,
skobkin commented 2023-07-20 18:21:36 +00:00
Outdated
Review
Copy link

What type? Why 25 exactly?

What type? Why 25 exactly?
👍 1
skobkin commented 2023-08-06 11:10:15 +00:00
Review
Copy link

Why 128?

Why 128?
#[ORM\Column(type: 'string', length: 128, nullable: false)] #[ORM\Column(type: 'string', length: 128, nullable: true)]
public readonly string $author, public readonly ?string $author,
skobkin marked this conversation as resolved Outdated
skobkin commented 2023-07-21 20:33:09 +00:00
Outdated
Review
Copy link

Why not nullable?

Why not nullable?
👍 1
#[ORM\Column(type: 'datetime_immutable', nullable: false)] #[ORM\Column(type: 'datetime_immutable', nullable: false)]
public readonly \DateTimeImmutable $publishDate, public readonly \DateTimeImmutable $publishDate,
#[ORM\Column(type: 'datetime_immutable', nullable: true)] #[ORM\Column(type: 'datetime_immutable', nullable: true)]

2
src/Form/Type/PasteForm.php
View file

@ -28,7 +28,7 @@ class PasteForm extends AbstractType
) )
->add('description', TextType::class, ['required' => false]) ->add('description', TextType::class, ['required' => false])
->add('text', TextareaType::class) ->add('text', TextareaType::class)
->add('author', TextType::class, ['attr' => ['maxlength' =>128]]) ->add('author', TextType::class, ['attr' => ['maxlength' => 128], 'required' => false])
skobkin marked this conversation as resolved Outdated
skobkin commented 2023-07-21 21:18:54 +00:00
Outdated
Review
Copy link

required => false?

`required => false`?
👍 1
->add('filename', TextType::class, ['required' => false, 'attr' => ['maxlength' =>128]]) ->add('filename', TextType::class, ['required' => false, 'attr' => ['maxlength' =>128]])
skobkin commented 2023-08-06 11:19:24 +00:00
Review
Copy link

Why 128?

Why 128?
->add('expirationDate', DateTimeType::class, [ ->add('expirationDate', DateTimeType::class, [
'required' => false, 'required' => false,