Feature/event validation #19

DankaMarci · 2024-05-28T15:34:37Z

It's nearly done, but I just can't seem to figure it out how to make the prisma compatible with the dtos.

csiszaralex · 2024-06-04T17:29:13Z

The build is not works, please check it

balintking · 2024-06-04T17:34:24Z

Yeah we talked bout it, but didn't have time to to check the problem in detail. There's something with the CreateEventDto.

csiszaralex · 2024-06-04T17:39:37Z

There are multiple problems:

You not installed the @nestjs/swagger, class-transformer and class-validator, but you use it
You receive CreateEventDto in event.service.ts create method, but prisma wait another type, please check it
The problem is same with update and UpdateEventDto

csiszaralex · 2024-06-04T17:40:56Z

I resolved the first problem 😉

Tschonti · 2024-06-04T17:52:37Z

apps/backend/src/event/entities/event.entity.ts

+  @IsOptional()
+  priority: Priority;
+
+  @IsEnum(Priority)


I assume this is incorrect

Yes, it should be @IsEnum(Status)

Tschonti · 2024-06-04T17:57:07Z

apps/backend/src/event/entities/event.entity.ts

+  @IsInt()
+  @IsNotEmpty()
+  categoryId: number;
+
+  @IsNotEmpty()
+  category: Category;
+
+  @IsInt()
+  @IsOptional()
+  ownerUserId: number;
+
+  @IsOptional()
+  ownerUser: User;
+
+  @IsInt()
+  @IsOptional()
+  ownerGroupId: number;
+
+  @IsOptional()
+  ownerGroup: Group;


Do not create a field for relations in the entity schema. This class should represent one record in the database. Prisma hides this from you, but there are no ownerGroup, ownerUser or category fields on the Event table (only categoryId, ownerUserId and ownerGroupId), so you should remove those from this class. When an event is created, the user, group and category likely already exists, and prisma will be able to link the new event record to those records with the id (foreign key) fields

…/collage-schedule-planner into feature/event-validation

csiszaralex

I marked some problems, please fix them and it will be perfect 😃

csiszaralex · 2024-06-05T15:45:06Z

apps/backend/package.json

    "@prisma/client": "^5.13.0",
+    "class-transformer": "^0.5.1",
+    "class-validator": "^0.14.1",
+<<<<<<< HEAD


Please resolve the merge conflict (just delete all between <<< and >>> 76...43).

csiszaralex · 2024-06-05T15:45:37Z

apps/backend/src/event/entities/event.entity.ts

+  @IsOptional()
+  priority: Priority;
+
+  @IsEnum(Priority)


Yes, it should be @IsEnum(Status)

balintking

My observations..

balintking · 2024-06-19T17:58:00Z

apps/backend/src/event/entities/event.entity.ts

+  @IsOptional()
+  categoryId: number;
+
+  category: Category;


Don't need category

apps/backend/src/event/entities/event.entity.ts

balintking · 2024-06-19T18:03:23Z

apps/backend/src/event/entities/event.entity.ts

+}
+export class Event {
+  @IsInt()
+  @IsOptional()


id should not be optional as I see

balintking · 2024-06-19T18:05:49Z

apps/backend/src/event/entities/event.entity.ts

+
+  @IsString()
+  @IsOptional()
+  color: string;


when optional we could declare as color?: string, so that it is nullable

balintking · 2024-06-19T18:11:17Z

apps/backend/src/event/dto/create-event.dto.ts

+
+import { Event } from '../entities/event.entity';
+
+export class CreateEventDto extends OmitType(Event, ['id', 'category', 'ownerGroupId', 'ownerUserId']) {}


I would say we need everything upon creation, but maybe I'm wrong.

balintking

Hi, sorry for the little late. Reviewed your code, please resolve those changes quickly and we can merge this branch. I tried to explain what you might misunderstood about the optional fields, but if it's still not clear, hit me up.
Yeah, and one more thing: why is the class category even in the event entity file?

balintking · 2024-06-21T17:47:49Z

apps/backend/src/event/entities/event.entity.ts

  name: string;

  @IsString()
-  @IsOptional()


@IsOptional() should stay as well, sorry if I wasn't clear

balintking · 2024-08-28T23:19:19Z

apps/backend/src/event/entities/event.entity.ts

+  location: string;
+
+  @IsDate()
+  @IsOptional()


start date is not optional

I think you mixed this up with the default values. But this is the entity, this should perfectly mirror what's in the database schema. The default values come handy creating an object and not telling those values. Thats what create dto is for.

balintking · 2024-08-28T23:20:58Z

apps/backend/src/event/entities/event.entity.ts

+  endTime: Date;
+
+  @IsEnum(Priority)
+  @IsOptional()


priority is also not optional according to the schema

balintking · 2024-08-28T23:21:31Z

apps/backend/src/event/entities/event.entity.ts

+  priority: Priority;
+
+  @IsEnum(Status)
+  @IsOptional()


not optional

balintking · 2024-08-28T23:26:22Z

apps/backend/src/event/entities/event.entity.ts

+  categoryId: number;
+
+  @IsInt()
+  ownerUserId: number;


balintking · 2024-08-28T23:26:29Z

apps/backend/src/event/entities/event.entity.ts

+  ownerUserId: number;
+
+  @IsInt()
+  ownerGroupId: number;


balintking · 2024-08-28T23:31:04Z

apps/backend/src/event/dto/create-event.dto.ts

+
+import { Event } from '../entities/event.entity';
+
+export class CreateEventDto extends OmitType(Event, ['ownerGroupId', 'ownerUserId']) {}


Omit only id, because that's the only information the database comes up with alone. The owner we should specify.

DankaMarci · 2024-09-27T12:39:29Z

I altered the event_has_only_one_owner constraint in the prisma migrations. As I see an event's ownership is optional in the prisma schema therefore (if I'm right) it should be optional in the migration as well.

I also made the events' category field optional. I don't think it's a must for an event to have a category.

DankaMarci added 2 commits May 28, 2024 17:19

Data validation

34b4677

Validation fixes

b3e83aa

DankaMarci requested review from csiszaralex and balintking May 28, 2024 15:34

balintking linked an issue Jun 1, 2024 that may be closed by this pull request

Add Class Validation to Event Entity #18

Open

4 tasks

Adds required packages

76a675a

csiszaralex mentioned this pull request Jun 4, 2024

Feature/User validation #20

Merged

Tschonti reviewed Jun 4, 2024

View reviewed changes

DankaMarci added 3 commits June 5, 2024 17:05

dto and entity fixes

39098df

rand fix

d3b6244

Merge branch 'feature/event-validation' of https://github.com/kir-dev…

3102e7a

…/collage-schedule-planner into feature/event-validation

csiszaralex requested changes Jun 5, 2024

View reviewed changes

Conflict and event entity fixes

9c5ce14

balintking requested changes Jun 19, 2024

View reviewed changes

rquested changes on the Event dto

4f05111

balintking requested changes Aug 28, 2024

View reviewed changes

Requested fixes on the Event DTO

ef883f1

Merge branch 'main' into feature/event-validation

59eadf9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/event validation #19

Feature/event validation #19

DankaMarci commented May 28, 2024

csiszaralex commented Jun 4, 2024

balintking commented Jun 4, 2024

csiszaralex commented Jun 4, 2024

csiszaralex commented Jun 4, 2024

Tschonti Jun 4, 2024

csiszaralex Jun 5, 2024

Tschonti Jun 4, 2024

csiszaralex left a comment

csiszaralex Jun 5, 2024

csiszaralex Jun 5, 2024

balintking left a comment

balintking Jun 19, 2024

balintking Jun 19, 2024

balintking Jun 19, 2024

balintking Jun 19, 2024

balintking left a comment •

edited

Loading

balintking Jun 21, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

balintking Aug 28, 2024

DankaMarci commented Sep 27, 2024


		import { Event } from '../entities/event.entity';

		export class CreateEventDto extends OmitType(Event, ['id', 'category', 'ownerGroupId', 'ownerUserId']) {}

Feature/event validation #19

Are you sure you want to change the base?

Feature/event validation #19

Conversation

DankaMarci commented May 28, 2024

csiszaralex commented Jun 4, 2024

balintking commented Jun 4, 2024

csiszaralex commented Jun 4, 2024

csiszaralex commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csiszaralex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balintking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balintking left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DankaMarci commented Sep 27, 2024

balintking left a comment •

edited

Loading