Uh oh!
There was an error while loading. Please reload this page.
Add TypeORM models - #162
Conversation
Zeko369
commented
Nov 9, 2019
You should defiantly add realtions on the ORM level |
eolculnamo2
commented
Nov 9, 2019
That’s what I thought. Will add. |
timmyichen
commented
Nov 10, 2019
thanks for adding these! just a quick note, model fields should be |
eolculnamo2
commented
Nov 16, 2019
Updated relationships and casing. Tag was missing event_id which is required for the one to many relationship. I've added that to the class. Also, I'm not 100% sure I understand the one-to-one relationships from sequelize-typescript (If someone could double check) :) |
vaibhavsingh97
left a comment
There was a problem hiding this comment.
@eolculnamo2 can you also update this PR with migration files.
@eolculnamo2 the schema was showing only one tag per event, which seems like an oversight. Is Discord, I just asked "The mock-ups already show multiple tags per event and it seems a very safe assumption 1 tag per event is not enough. That said, do we need an MVP user story that says 'As an Admin, I can apply optional tags to an event'? Or, do we think this isn't a necessary MVP story. If not, then should we still building an event_tags table now to show the future intention of multiple tags?" |
eolculnamo2
commented
Nov 17, 2019
The event tags decision is probably not my call, but it seems simple enough to include with the MVP and its already in our mock ups. Is there a way to automatically build the migrations file from what I already have or does that need to be manually typed? (I ask because it reminds me of C#'s Entity Framework which build it for you) |
timmyichen
commented
Nov 17, 2019
As far as I know there is no way to automatically generate migrations file. If we were using non-typescript sequelize it would be fairly straightforward as the migration file would look almost identical to the model declaration object, but since we are using typescript-sequelize, the model declarations are classes. |
Zeko369
commented
Nov 18, 2019
@timmyichen@eolculnamo2 If we used |
vkWeb
commented
Nov 18, 2019
I agree with @allella that we need multiple tags per event and it should be in MVP. We can control the maximum number of tags organizers can apply client side (or server-side). I am updating the database, er diagram and api schemas as well. I'll send a PR soon. @timmyichen, @Zeko369, @nik-john, @allella I was looking at the api routes and I realized that the routes are revolving around |
vkWeb
commented
Nov 18, 2019
Blocking this till we resolve the architectural issues on #251. |
Unless we're not going to show tags at all in the MVP, then I'd also vote to make the multi-tags changes to the schema, which I believe @vkWeb is going to submit as a PR. Tags were implied to be an MVP feature based on earlier conversations and as evident by the design mockups, but I haven't seen a user story about tags. Anybody have an issue with creating a user story like 'As an Organizer, I can apply optional tags to an event' ? |
timmyichen
commented
Nov 18, 2019
@Zeko369 I'm down to use TypeORM but we should loop @QuincyLarson into this conversation as I believe it was he who initially proposed sequelize. |
Blocked because of #252, resolved -> USING TYPEORM |
Zeko369
commented
Nov 21, 2019
We're switching to So this is blocked till #257 is merged |
eolculnamo2
commented
Nov 22, 2019
Awesome, will get this done soon! I'll shoot to have it done sometime tomorrow or the next day. |
eolculnamo2
commented
Nov 26, 2019
Things have come up lately that have made it hard to finish this in a timely manner :/ |
Zeko369
commented
Nov 26, 2019
@eolculnamo2 When I finish TypeORM migration PR #257 I'll take over this |
ankorGH
commented
Nov 29, 2019
@Zeko369 , if you are not working on it I would like to take a go at it. |
Zeko369
commented
Nov 29, 2019
Today I'll finish the dev docs on TypeORM and how we're using it so I'll tag you in the pr, and then you can finish this migration @ankorGH |
ankorGH
commented
Nov 29, 2019
sure |
vaibhavsingh97
commented
Dec 6, 2019
Zeko369
commented
Dec 6, 2019
@ankorGH If you don't have enough time, I can do this tomorrow |
ankorGH
commented
Dec 7, 2019
@Zeko369 I am on it. |
ankorGH
commented
Dec 8, 2019
@Zeko369@eolculnamo2 can you grant me access to make commits to this PR or I should create a new PR |
Zeko369
commented
Dec 8, 2019
@ankorGH I think that for the sake of simplicity and to get it done sooner it would be better to just create a new PR from your fork |
Zeko369
commented
Dec 8, 2019
Closing and moving to #287 |
Update README.md).masterbranch of Chapter.Adds sequelize models as show in the db diagram. Potential improvement would be to use HasMany relationships over simply referencing the Id in some case.