pokemontrades / porygon-bot

Porygon-Bot for IRC.
Apache License 2.0
7 stars 5 forks source link

Add quotes command #69

Closed AlMcKinlay closed 4 years ago

AlMcKinlay commented 7 years ago
AlMcKinlay commented 7 years ago

Table info

CREATE TABLE `Quote` (
  `ID` int(11) NOT NULL  AUTO_INCREMENT PRIMARY KEY,
  `Author` text NOT NULL,
  `Message` text NOT NULL,
  `Chan` text NOT NULL
)
AlMcKinlay commented 6 years ago

Any interest in this PR? If not, I'll close it and just keep it on my fork for my version. I just want to tidy up the number of branches I have.

robdy commented 6 years ago

@McInkay, what do you think about returning some message if someone use get but there are no quotes? Like that:

if (result) {
  return `${result.Message} (submitted by ${result.Author})`;
}
else {
  return 'No quotes for this channel';
};

Anyway, looks fine to me. Tested all the commands, no issue found.

AlMcKinlay commented 6 years ago

@robdy Hmm, does that not happen?

return result ? `${result.Message} (submitted by ${result.Author})` : `No results for ${quote}`;

This should do that, shouldn't it? Or am I misunderstanding what you are suggesting?

AlMcKinlay commented 6 years ago

I think I'll refactor this because I'm not happy with how the code is laid out as it's been so long since I implemented it :-P

robdy commented 6 years ago

I mean if you want to get the random one by .quote get when there are no quotes in the database it'll skip if (quote) (as quote is undefined), go to

return db.conn.query('SELECT * FROM Quote WHERE Quote.Chan = ? ORDER BY RAND() LIMIT 1', [channel]).get(0).then(result => {
  return `${result.Message} (submitted by ${result.Author})`

and return error only in the console.

AlMcKinlay commented 6 years ago

Oh duh, I was confused because the "get" was indented incorrectly. Aye, I'll add that.

AlMcKinlay commented 6 years ago

I have refactored this and added the message if there are no quotes and you ask for a random one.

robdy commented 6 years ago

LGTM

AlMcKinlay commented 6 years ago

I've updated but I still need to test it. I think I'll get sqlite in before we merge this one because it makes it much simpler to test. I've got 1 more PR to do (just about to put it up) before I put up sqlite.