WYHNUS / ExchangeBuddy

Find your exchange buddies!
5 stars 0 forks source link

Clean up Universities table #137

Open irvinlim opened 7 years ago

irvinlim commented 7 years ago

I see there are quite a number of columns which were reused from the original Assignment 1 project. Can I check if we intend to use any of the following columns?

We might want to keep the topUnisId column if we wish to make use of the data found on topuniversities.com to fetch wiki data or something.

ZhangHanming commented 7 years ago

I suggest that we remove linkUrl, CountryAlpha2Code and logoUrl as they are repetitive, terms as we indicate by month not terms, and emailDomains as it's hard to bootstrap this data?

Also what bgImageUrlis used for? Guess we can remove it also? @WYHNUS

WYHNUS commented 7 years ago

bgImageUrl is background image for University, which is now default to the same image for all Uni, but useful in the future. emailDomains is potentially useful in the future for verification of student's validity (I guess). Let's keep those two there for now.

I will check other fields when I'm back home, and will update schema accordingly. :)

WYHNUS commented 7 years ago

Let's remove linkUrl , logoUrl and countryCode

Reason being, the first two are duplicated and neither countryCode nor CountryAlpha2Code should be present in the University table as it is linked with Country table.

@irvinlim Do you think topUnisId is necessary, I don't see how we are going to use that though... Meanwhile, CountryAlpha2Code is not present in University table, it is a primary key for Country.

I suggest we should keep emailDomains column as it is potential useful in the future. But do you guys think we will use terms column? @plsgibchance

WYHNUS commented 7 years ago

@ZhangHanming Can help to do data migration to

  1. drop the specified columns
  2. linkUniversity and Country tables together
irvinlim commented 7 years ago

@ZhangHanming Will need you to link University and Country tables together for production before we launch v2.0, in order for me to fix #111.

irvinlim commented 7 years ago

@WYHNUS If we don't include CountryAlpha2Code as a foreign key in Universities table, then how do you link them?

WYHNUS commented 7 years ago

Use Sequelize's association which is already defined in DB but not in use for University ...