Add Broadcast, Subscribe, Bus, Weather and unit tests(with Spaces) #51

pengnam · 2017-12-22T19:29:42Z

New Changes:

Functions added

Bus (refactored)
Weather (refactored)
Broadcast (new)

Broadcast should be used as followed: call /broadcast with tags, valid tags will be returned in a mark-up message, and the subsequent message to that will be forwarded to all users who are subscribed to that tag

Subscribe (new) (+ Unsubscribe)

After performing a series of checks, subscribes the user to the tag

Spaces (from Cheng Yao) - I have made some edits based on the comments that you gave him two days ago. I have worked on some of them and the rest I will talk to Cheng Yao about it.
help, feedback, cbs
Fixed the repeated user bug! Sadly that took me some time.
NUS Bus (new) - NUS Bus Timings.

Features added:
Implemented a cache.
Cache is a field under cinnabot so I can refer to it. It is used in cb.Router only cache last function called.

Current problems:

[I am unsure about my approach to testing as we have not talked about it specifically for these functions before]
1) Problems with unit test for bus and weather
I attempted the use the technique in section 1: Monkey Patching.(Refer to https://husobee.github.io/golang/testing/unit-test/2015/06/08/golang-unit-testing.html) This is to "monkey-patch" the http request as I felt that it was not important to that test. However, the monkey patch requires me to declare a variable. (Refer to line before func() main in image.)

If I do not declare a separate variable:

Hence, is it right for me to monkey-patch so as to not make a URL request? And is monkey-patch the right way to do it?

2) Problems with unit test for broadcast [SOLVED]

Minor notes:

I should have committed early, often, and using lower-case.
I apologise for slightly messy code. I should have cleaned it up proper but I thought that getting your feedback on what I am doing so far is more crucial to the project's success.
I should have better error handling but Im still not sure about go's way of handling errors/exceptions(or the lack of it).

varunpatro · 2018-02-21T17:20:35Z

Dockerfile

@@ -0,0 +1,39 @@
+FROM golang:1.9


I suppose we don't need this file anymore.

varunpatro · 2018-02-21T17:24:12Z

main/.#main.go

@@ -0,0 +1 @@
+[email protected]


add this file to gitignore!

varunpatro · 2018-02-21T17:26:05Z

model/db.go

+	CheckTagExists (id int, tag string) bool
+	CheckSubscribed (id int, tag string) bool
+	UpdateTag (id int, tag string, flag string) error
+


remove newline

varunpatro · 2018-02-21T17:29:05Z

model/user.go

@@ -2,13 +2,42 @@ package model

 import (
 	"github.com/jinzhu/gorm"


stdlib should appear before 3rd party

varunpatro · 2018-02-21T17:29:28Z

model/user.go

+	UserID    int `gorm:"primary_key"`
+	CreatedAt time.Time
+	UpdatedAt time.Time
+	//DeletedAt time.Time


remove the commented line?

varunpatro · 2018-02-21T18:00:32Z

basic.go

 }

 // About returns a link to Cinnabot's source code.
 func (cb *Cinnabot) About(msg *message) {
-	cb.SendTextMessage(msg.From.ID, "Touch me: https://github.com/varunpatro/Cinnabot")
+	cb.SendTextMessage(int(msg.Chat.ID), "Touch me: https://github.com/varunpatro/Cinnabot")


You should add yourself here too! Say it was developed by you too (before showing the link)

varunpatro · 2018-02-21T18:01:05Z

basic.go

+//Link returns useful resources
+func (cb *Cinnabot) Resources(msg *message) {
+	resources := make(map[string]string)
+	resources["usplife"] = "[fb page](https://www.facebook.com/groups/usplife/)"


use const for strings

Do you mean:
const usplife = "testString"
resources["usplife"] = usplife

yeah but more descriptive const names such as

const ( uspFacebookUrl := "https://wwww.facebook.com/groups/usplife/" uspSpacesUrl := "http://www.nususc.com/Spaces.aspx" )

varunpatro · 2018-02-21T18:01:19Z

basic.go

+	}
+
+	//Default loc: Cinnamon
+	loc := &tgbotapi.Location{Latitude: 1.306671, Longitude: 103.773556}


Constant can only be declared on character, string, boolean values, so I can only make the latitude and longitude values a constant value.
Since this is the only only use of it in this file, where should I declare the constant? And how should I do that.

declare two consts at the top of the file:

const ( cinnamonLat := 1.306671 cinnamonLong := 103.773556 )

varunpatro · 2018-02-21T18:01:43Z

basic.go

+	client := &http.Client{}
+
+	req, _ := http.NewRequest("GET", "https://api.data.gov.sg/v1/environment/2-hour-weather-forecast", nil)
+	req.Header.Set("api-key", "d1Y8YtThOpkE5QUfQZmvuA3ktrHa1uWP")


Uh oh. Cardinal sin here. Do not check in sensitive info (api keys) into VCS

varunpatro · 2018-02-21T18:02:45Z

basic.go

+		"*Subscribe status*\n" + "(sub status) tag: description\n"
+	for i := 0; i < len(cb.allTags); i += 2 {
+		if cb.db.CheckSubscribed(msg.From.ID, cb.allTags[i]) {
+			listText += "✅" + cb.allTags[i] + " : " + cb.allTags[i+1] + "\n"


space after the emoji would be great!

to me, a green cross is not the best for unsubscribed status. What about the following pairs:

a/ ✅❌
b/ ✅✖️
c/ ✔️✖️

…ot-go" This reverts commit 52c832c, reversing changes made to 955b68a.

Update handle for study groups resources

…o cinnabot-go

Created a dining hall survey function

* Add function for bot stats * Refactor code to separate files for feedback and broadcast * Add feedback for OHS * Refine and finish stats function * Remove useless log print in echo * Better responses for stats function

…into cinnabot-go

…innabot into cinnabot-go" This reverts commit cbfbfd8, reversing changes made to bcd4e7c.

…ot-go

Sean and others added 30 commits September 20, 2017 00:29

add capitalise

79acbdb

Merge remote-tracking branch 'upstream/cinnabot-go' into cinnabot-go

a2de806

Refactor weather/bus

1122c5a

Add subscribe and broadcast feature

dcca8c9

Add spaces.go

f33e760

add test for spaces tried test for weather, bus, broadcast

d661903

edit after comments on spaces

b4363b0

shift db as field in cb

976ae41

resolve unique user bug

8dd0122

fix bug unique users using another way

8db765e

solve subscribe not update bug

6fd4a52

add feedback, unsubscribe, cbs, feedback, help

4b828a7

add Dockerfile

494c956

fix mockdb dependency injection bug

bd245f3

add subscribe and unsubscribe test

ab3af5e

add different feedback functions

f7304b0

add start

c6caddb

update help and link

b5c757c

add list of nus stops in json

b670129

modify fields in json

9c2a4dd

modify field types in json

7a52956

add NUSBus func

1f87424

remove env variable for main use cb.db instead

e0bfa90

remove env variable for main use cb.db instead(2)

b6606b1

add cache to allow easy retrieval

e8ee36c

modify bus and feedback functions to work with cache

10d6808

fix nusbus function

074c7d3

solve cache bug

221f44c

polish weather and nusbus

ff3a356

shift bus and nusbus out to seperate file

18fa4e8

seanngpengnam added 2 commits February 21, 2018 15:26

add law button for nusbus and reshuffle

39f5657

configure mechanics of everything

88bea43

varunpatro reviewed Feb 21, 2018

View reviewed changes

seanngpengnam and others added 27 commits February 22, 2018 12:15

remove adding 480

d445b47

edit after comments

ee8f876

Add all tags check before sending message

b69680a

Formatting and changing fatal logging to print logging

c43a453

UserGroup returns all values if all tag is used

30f0c35

Remove adding 480 to bus timing response, change log fatal to log print

955b68a

Merge remote-tracking branch 'origin/cinnabot-go' into cinnabot-go

52c832c

Revert "Merge remote-tracking branch 'origin/cinnabot-go' into cinnab…

95e984c

…ot-go" This reverts commit 52c832c, reversing changes made to 955b68a.

Reminder to include tag at the start of the message

888f876

Update handle for study groups resources

bf1c2cb

Merge pull request #1 from ningyuansg/update-uyp-usphonebook

95b7f0b

Update handle for study groups resources

change events default to true

48fa551

Merge branch 'cinnabot-go' of https://github.com/pengnam/cinnabot int…

61caa32

…o cinnabot-go

Created function for DH Survey

9c84df9

Add feedback to database

d5ab0b6

Added error handling in DH survey function

5c9faeb

Change questions for DH Feedback

ebb203d

Merge pull request #3 from chaitanyabaranwal/cinnabot-go

28b4cca

Created a dining hall survey function

Resolve package naming error from merge

4a5002d

Uncomment feedback db creation statement

8b602fd

Amend travis

f127806

Finish stats function (#9)

bcd4e7c

* Add function for bot stats * Refactor code to separate files for feedback and broadcast * Add feedback for OHS * Refine and finish stats function * Remove useless log print in echo * Better responses for stats function

Debug stats functions in db.go (#11)

11c38f4

Merge branch 'cinnabot-go' of https://github.com/varunpatro/cinnabot …

cbfbfd8

…into cinnabot-go

Revert "Merge branch 'cinnabot-go' of https://github.com/varunpatro/c…

0ad70b9

…innabot into cinnabot-go" This reverts commit cbfbfd8, reversing changes made to bcd4e7c.

Merge branch 'cinnabot-go' of github.com:pengnam/cinnabot into cinnab…

692f0ae

…ot-go

Apply golint, revert back path names

0ed76fc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Broadcast, Subscribe, Bus, Weather and unit tests(with Spaces) #51

Add Broadcast, Subscribe, Bus, Weather and unit tests(with Spaces) #51

pengnam commented Dec 22, 2017 •

edited

Loading

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

pengnam Mar 1, 2018

varunpatro Mar 1, 2018

varunpatro Feb 21, 2018

pengnam Mar 1, 2018 •

edited

Loading

varunpatro Mar 1, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

varunpatro Feb 21, 2018

		@@ -2,13 +2,42 @@ package model

		import (
		"github.com/jinzhu/gorm"

Add Broadcast, Subscribe, Bus, Weather and unit tests(with Spaces) #51

Are you sure you want to change the base?

Add Broadcast, Subscribe, Bus, Weather and unit tests(with Spaces) #51

Conversation

pengnam commented Dec 22, 2017 • edited Loading

New Changes:

Current problems:

Minor notes:

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

Choose a reason for hiding this comment

pengnam Mar 1, 2018 • 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

pengnam commented Dec 22, 2017 •

edited

Loading

pengnam Mar 1, 2018 •

edited

Loading