added tag model #79

adesouky · 2018-11-10T13:44:03Z

🎟️ Ticket(s): Closes #49

👷 Changes

Added tag models and create, get, delete, and change name operations

🔦 Testing Instructions

go to core/database and run "go test -run TestTag"

codecov · 2018-11-10T13:48:22Z

Codecov Report

Merging #79 into master will decrease coverage by 0.13%.
The diff coverage is 77.78%.

@@            Coverage Diff             @@
##           master      #79      +/-   ##
==========================================
- Coverage   80.57%   80.44%   -0.12%     
==========================================
  Files          23       24       +1     
  Lines         746      782      +36     
  Branches        4        4              
==========================================
+ Hits          601      629      +28     
- Misses        105      109       +4     
- Partials       40       44       +4

Flag	Coverage Δ
#client	`100% <ø> (ø)`	⬆️
#golang	`79.74% <77.78%> (-0.09%)`	⬇️

Impacted Files	Coverage Δ
core/database/tag.go	`77.78% <77.78%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b5e25d...ccac996. Read the comment docs.

bobheadxi

wow thanks for getting this PR in! 😍

first of all, make sure you have a gofmt plugin installed on your IDE - I recommend using Visual Studio Code alongside the official Go plugin

it'll help you fix a lot of common errors (at a glance I see a few) and help keep our code consistent!

adesouky · 2018-11-10T13:55:16Z

Yea that sounds so helpful ill definitely use it.
Upon testing, I found that the change name test is inconsistent.
Although the update operation goes through, the name change is not consistent. i.e it will change the name on one test run, but not on the following. I have no idea why though!

bobheadxi · 2018-11-10T13:56:58Z

@adesouky hm interesting...

although on that front, I think there might have been a slight misunderstanding - there shouldn't be a need for a name change function, just the ability to add and remove tags from an applicant

we can clarify in a few hours, get some rest! 😁

adesouky · 2018-11-10T14:00:08Z

Well good to know 😂

bobheadxi

Awesome work! 🎉 See if you can model the tests after the new tests the way we talked about them yesterday! Great use of the go test -run TestTag suggestion as well

I think @srijonsaha should have a look at this too before we land this PR

Thanks! 😁

core/database/tag.go

core/database/database_test.go

bobheadxi · 2018-11-11T19:43:34Z

Oh yeah and don't worry about the merge conflicts too much - I can handle them for you before you merge if you want (but I recommend you give it a try locally! just run git pull origin master)

core/database/tag.go

bobheadxi · 2018-11-12T07:39:26Z

another thing you could do is just submit this change and I can work off this commit and integrate it will the clubtable

I'll leave this up to @adesouky ! Anas, just add the "Ready for Review" label back to this PR when you've addressed the comments regarding the tests and the code cleanup (comments, etc.) - up to you if you want to add the changes for integrating with the club table - and I'll fix up the merge conflicts and we can merge this 😁

bobheadxi

Awesome work cleaning this up @adesouky ! 😍

Left one super minor comment regarding naming - once that's fixed, I think we're good to merge this 😁

bobheadxi · 2018-11-21T19:10:29Z

core/database/tag.go

+}
+
+// GetTag gets the tag associated with Applicant_ID, Period_ID, & Event_ID
+func (db *Database) GetTag(ApplicantID string, PeriodID string, EventID string, c *model.Club) (*model.Tag, error) {


in Go, variable names should be lowercased, as uppercased names have a special meaning in a lot of contexts (exports)

Suggested change

func (db *Database) GetTag(ApplicantID string, PeriodID string, EventID string, c *model.Club) (*model.Tag, error) {

func (db *Database) GetTag(applicantID string, periodID string, eventID string, c *model.Club) (*model.Tag, error) {

bobheadxi · 2018-11-21T19:10:50Z

core/database/tag_test.go

+			&model.Club{
+				ID:          "1234",
+				Name:        "Launchpad",
+				Description: "1337 h4x0r",


bobheadxi · 2018-11-21T19:11:23Z

core/model/tag.go

@@ -0,0 +1,8 @@
+package model
+
+// Tag model


succinct, but it works 😅

bobheadxi

great stuff, thanks @adesouky !

bobheadxi · 2018-11-22T02:33:23Z

@ubclaunchpad/pinpoint as always, please take a look when yall have a chance

added tag model

bfc8607

adesouky requested a review from bobheadxi as a code owner November 10, 2018 13:44

adesouky requested a review from a team November 10, 2018 13:44

bobheadxi reviewed Nov 10, 2018

View reviewed changes

bobheadxi added this to In progress in Backend MVP via automation Nov 10, 2018

bobheadxi added the work in progress label Nov 10, 2018

adesouky added 2 commits November 10, 2018 06:11

removed unnecessary function

f369fa2

fixed formatting errors

4dea856

adesouky added the ready for review label Nov 11, 2018

adesouky moved this from In progress to Needs review in Backend MVP Nov 11, 2018

bobheadxi suggested changes Nov 11, 2018

View reviewed changes

core/database/tag.go Outdated Show resolved Hide resolved

core/database/tag.go Show resolved Hide resolved

core/database/database_test.go Outdated Show resolved Hide resolved

core/database/database_test.go Outdated Show resolved Hide resolved

bobheadxi requested review from srijonsaha-zz and a team November 11, 2018 19:43

bobheadxi reviewed Nov 12, 2018

View reviewed changes

core/database/tag.go Outdated Show resolved Hide resolved

bobheadxi removed the ready for review label Nov 12, 2018

adesouky added 2 commits November 21, 2018 02:35

merge

1bc9f16

Added suggested changes

dd95e74

adesouky added ready for review and removed work in progress labels Nov 21, 2018

adesouky requested review from bobheadxi and srijonsaha-zz November 21, 2018 11:31

bobheadxi reviewed Nov 21, 2018

View reviewed changes

bobheadxi requested a review from a team November 21, 2018 19:12

lowercased names

ccac996

bobheadxi approved these changes Nov 22, 2018

View reviewed changes

Backend MVP automation moved this from Needs review to Reviewer approved Nov 22, 2018

bobheadxi merged commit 3d26b50 into master Nov 22, 2018

Backend MVP automation moved this from Reviewer approved to Done Nov 22, 2018

bobheadxi deleted the tag_model branch November 22, 2018 02:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added tag model #79

added tag model #79

adesouky commented Nov 10, 2018

codecov bot commented Nov 10, 2018 •

edited

bobheadxi left a comment

adesouky commented Nov 10, 2018

bobheadxi commented Nov 10, 2018 •

edited

adesouky commented Nov 10, 2018

bobheadxi left a comment •

edited

bobheadxi commented Nov 11, 2018

bobheadxi commented Nov 12, 2018

bobheadxi left a comment

bobheadxi Nov 21, 2018

adesouky Nov 22, 2018

bobheadxi Nov 21, 2018

bobheadxi Nov 21, 2018

bobheadxi left a comment

bobheadxi commented Nov 22, 2018

	func (db Database) GetTag(ApplicantID string, PeriodID string, EventID string, c model.Club) (*model.Tag, error) {
	func (db Database) GetTag(applicantID string, periodID string, eventID string, c model.Club) (*model.Tag, error) {

Navigation Menu

added tag model #79

added tag model #79

Conversation

adesouky commented Nov 10, 2018

👷 Changes

🔦 Testing Instructions

codecov bot commented Nov 10, 2018 • edited

Codecov Report

bobheadxi left a comment

Choose a reason for hiding this comment

adesouky commented Nov 10, 2018

bobheadxi commented Nov 10, 2018 • edited

adesouky commented Nov 10, 2018

bobheadxi left a comment • edited

Choose a reason for hiding this comment

bobheadxi commented Nov 11, 2018

bobheadxi commented Nov 12, 2018

bobheadxi left a comment

Choose a reason for hiding this comment

bobheadxi Nov 21, 2018

Choose a reason for hiding this comment

adesouky Nov 22, 2018

Choose a reason for hiding this comment

bobheadxi Nov 21, 2018

Choose a reason for hiding this comment

bobheadxi Nov 21, 2018

Choose a reason for hiding this comment

bobheadxi left a comment

Choose a reason for hiding this comment

bobheadxi commented Nov 22, 2018

codecov bot commented Nov 10, 2018 •

edited

bobheadxi commented Nov 10, 2018 •

edited

bobheadxi left a comment •

edited