Conversation
|
New commit merges "flag" and "multiFlag" into a single trigger type. Currently defined triggers should work as they always did, but now there is only a single code flow |
OliPro007
left a comment
OliPro007
left a comment
There was a problem hiding this comment.
I have managed to deploy discourse and askgod using docker compose and setting up a test team and some test posts/flags, and this has worked properly (3 flags/tags, threshold 2, the topic appeared after 2 out of the 3 flags mentioned were submitted). Other topics appear correctly, though I haven't done an exhaustive test
| Tags []string `yaml:"tags"` | ||
| Threshold int64 `yaml:"threshold"` |
There was a problem hiding this comment.
Lack of alignment is going to tick off CI :)
| } | ||
| teams = append(teams, team) | ||
| } | ||
| } else if post.Trigger.Type == "multiFlag" { |
There was a problem hiding this comment.
I think I'd call it multi_flag instead as we've generally been doing snake case for API stuff at least
| if nTriggers < post.Trigger.Threshold { | ||
| continue | ||
| } | ||
| } |
| if int64InSlice(team.AskgodID, askgodFlags[tag]) { | ||
| nTriggers++ | ||
| } | ||
| } |
| @@ -102,15 +102,34 @@ type post struct { | |||
| } | |||
There was a problem hiding this comment.
We should squash this commit into the previous one to make it easier to review as this mostly supersedes the logic from the previous commit
|
LGTM. I'm currently doing a bunch of spring cleaning on the codebase but I'll then pick this up, fix the few things I noted in review and merge it. |
3 participants
Adds a discourse trigger that requires multiple flags to be submitted to trigger. The optional threshold attributes allow requiring only a certain number of flags instead of all of them.
Note: I do not have a setup to test discourse integration, so while it builds, I have not tested it.