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

1087 feature add ability to assign legal entitycompany as organizer #362

Conversation

dimitur2204
Copy link
Contributor

@dimitur2204 dimitur2204 commented Nov 17, 2022

Purpose
Add company_id field on the campaign regarding #1087 with a relation to the company field.

Notes
I noticed that sometimes we have these Relation objects and sometimes we don't
Here we have it:
image
And on this PR for example for the campaign we instead have a toEntity() function that seems to connect the fields:
image

Is there a difference?

@dimitur2204 dimitur2204 changed the base branch from master to schema-upgrade November 17, 2022 20:51
@dimitur2204 dimitur2204 changed the base branch from schema-upgrade to master November 17, 2022 20:51
@github-actions
Copy link

github-actions bot commented Nov 17, 2022

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.76% (+0.02% 🔼)
1903/2652
🔴 Branches 46.44% 248/534
🔴 Functions 44.49% 222/499
🟡 Lines
69.87% (+0.02% 🔼)
1688/2416

Test suite run success

180 tests passing in 64 suites.

Report generated by 🧪jest coverage report action from d9c3a0b

@dimitur2204 dimitur2204 self-assigned this Nov 18, 2022
@dimitur2204 dimitur2204 requested a review from kachar November 18, 2022 16:47
@dimitur2204 dimitur2204 marked this pull request as ready for review November 18, 2022 16:48
Copy link
Member

@kachar kachar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again an awesome contribution @dimitur2204

I can't see anything that can be improved around the code.

In your question - we're using toEntity so we can encapsulate which field are used to connect to external entity in the db. We validate the input uuids and link them annoyingly to reduce the duplication and reuse the code. The relation objects are a pattern that we couldn't validate enough to make sure we're consistent with it everywhere. The other approach seems to be more simplistic for the time being as the frontend doesn't have to know the proper json to match the Prisma linking style.

@dimitur2204 dimitur2204 merged commit 5c3540c into master Nov 19, 2022
@dimitur2204 dimitur2204 deleted the 1087-feature-add-ability-to-assign-legal-entitycompany-as-organizer branch November 19, 2022 16:51
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.

[Feature] Add ability to assign Legal Entity(Company) as Organizer
2 participants