From 59138e18ac3bb6475b7e7d1f8e0e6d9b40ae1289 Mon Sep 17 00:00:00 2001 From: Roman Vaivod Date: Tue, 31 Mar 2020 12:34:07 +0200 Subject: [PATCH 1/4] gh-133: Fix noImplicitAny errors --- backend/app.ts | 9 +-- backend/bot/bot-factory.ts | 12 ++-- backend/errors/BadRequestError.ts | 2 +- backend/errors/ConflictError.ts | 2 +- backend/errors/InputError.ts | 2 +- backend/errors/NotFoundError.ts | 2 +- backend/match-reporter/MatchReporter.ts | 26 ++++---- backend/package-lock.json | 6 ++ backend/package.json | 1 + backend/rating/rating-calculator.ts | 6 +- backend/repositories/MatchRepository.ts | 14 ++--- backend/storage/Storage.ts | 6 +- backend/storage/StorageContext.ts | 18 +++--- backend/storage/db/db-config.ts | 4 +- backend/storage/db/db-errors.ts | 8 ++- backend/storage/db/db-transactions.ts | 11 ++-- backend/storage/db/db-transformations.ts | 61 +++++++++++++------ backend/tsconfig.json | 20 +++--- backend/types/Database.ts | 77 ++++++++++++++++++++++++ backend/types/Game.ts | 8 ++- backend/types/Player.ts | 10 ++- backend/types/slackbots/index.d.ts | 20 ++++++ 22 files changed, 239 insertions(+), 86 deletions(-) create mode 100644 backend/types/Database.ts create mode 100644 backend/types/slackbots/index.d.ts diff --git a/backend/app.ts b/backend/app.ts index c174f3fe..7c01cdcb 100644 --- a/backend/app.ts +++ b/backend/app.ts @@ -1,5 +1,5 @@ import * as express from 'express' -import { Response, Request } from 'express' +import { NextFunction, Response, Request } from 'express' import * as bodyParser from 'body-parser' import * as storage from './storage/Storage' @@ -10,6 +10,7 @@ import { addGame } from './repositories/GameRepository' import { makeBot, SingleChannelBot } from './bot/bot-factory' import { MatchReporter } from './match-reporter/MatchReporter' +import { InputError } from './errors/InputError' const jsonParser = bodyParser.json() const urlencodedParser = bodyParser.urlencoded({ extended: false }) @@ -17,7 +18,7 @@ const urlencodedParser = bodyParser.urlencoded({ extended: false }) const app = express() const port = 3000 -const addCrossDomainHeaders = function(req, res, next): void { +const addCrossDomainHeaders = function(req: Request, res: Response, next: NextFunction): void { res.header('Access-Control-Allow-Origin', '*') res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE') res.header('Access-Control-Allow-Headers', 'Content-Type') @@ -28,7 +29,7 @@ app.use(addCrossDomainHeaders) app.use(urlencodedParser) app.use(jsonParser) -let matchReporter +let matchReporter: MatchReporter makeBot(process.env.FOOSBOT_TOKEN, process.env.FOOS_CHANNEL_NAME) .then((bot: SingleChannelBot) => { matchReporter = new MatchReporter(bot, process.env.MATCH_REPORT_PREFIX_SUFFIX_CONFIG) @@ -36,7 +37,7 @@ makeBot(process.env.FOOSBOT_TOKEN, process.env.FOOS_CHANNEL_NAME) }) .catch(error => console.warn('Slackbot initialization failed:', error.message)) -const processError = (response, error): void => { +const processError = (response: Response, error: InputError): void => { console.error(error) response.statusCode = error.httpStatusCode || 500 response.send(error.message) diff --git a/backend/bot/bot-factory.ts b/backend/bot/bot-factory.ts index 5fa837cb..8323415e 100644 --- a/backend/bot/bot-factory.ts +++ b/backend/bot/bot-factory.ts @@ -3,21 +3,21 @@ import SlackBot from 'slackbots' export class SingleChannelBot { constructor( private readonly slackbot: SlackBot, - private readonly channelName, - private readonly channelId + private readonly channelName: string, + private readonly channelId: string ) {} - setGroupPurpose(purpose): Promise { + setGroupPurpose(purpose: string): Promise { return this.slackbot._api('groups.setPurpose', { channel: this.channelId, purpose }) } - postMessage(message, isAsUser = true): Promise { + postMessage(message: string, isAsUser = true): Promise { /* eslint-disable-next-line @typescript-eslint/camelcase */ return this.slackbot.postTo(this.channelName, message, { as_user: isAsUser }) } } -export const makeBot = (botToken, channelName): Promise => { +export const makeBot = (botToken: string, channelName: string): Promise => { return new Promise((resolve, reject): void => { if (!botToken || !channelName) { reject(Error('botToken or channelName missing')) @@ -27,7 +27,7 @@ export const makeBot = (botToken, channelName): Promise => { token: botToken, }) - slackbot.on('error', error => reject(error)) + slackbot.on('error', (error: Error) => reject(error)) slackbot.on('start', async () => { let groupId diff --git a/backend/errors/BadRequestError.ts b/backend/errors/BadRequestError.ts index 1fbbefaa..b757cdee 100644 --- a/backend/errors/BadRequestError.ts +++ b/backend/errors/BadRequestError.ts @@ -1,6 +1,6 @@ export class BadRequestError extends Error { readonly httpStatusCode: number - constructor(message) { + constructor(message: string) { super(message) this.httpStatusCode = 400 } diff --git a/backend/errors/ConflictError.ts b/backend/errors/ConflictError.ts index 282ad0cd..903ce7c7 100644 --- a/backend/errors/ConflictError.ts +++ b/backend/errors/ConflictError.ts @@ -1,6 +1,6 @@ export class ConflictError extends Error { readonly httpStatusCode: number - constructor(message) { + constructor(message: string) { super(message) this.httpStatusCode = 409 } diff --git a/backend/errors/InputError.ts b/backend/errors/InputError.ts index 74eaf6f0..e0751faa 100644 --- a/backend/errors/InputError.ts +++ b/backend/errors/InputError.ts @@ -1,6 +1,6 @@ export class InputError extends Error { readonly httpStatusCode: number - constructor(message) { + constructor(message: string) { super(message) this.httpStatusCode = 422 } diff --git a/backend/errors/NotFoundError.ts b/backend/errors/NotFoundError.ts index 77a9bdab..77ca622f 100644 --- a/backend/errors/NotFoundError.ts +++ b/backend/errors/NotFoundError.ts @@ -1,6 +1,6 @@ export class NotFoundError extends Error { readonly httpStatusCode: number - constructor(message) { + constructor(message: string) { super(message) this.httpStatusCode = 404 } diff --git a/backend/match-reporter/MatchReporter.ts b/backend/match-reporter/MatchReporter.ts index 387fd819..67c8aff4 100644 --- a/backend/match-reporter/MatchReporter.ts +++ b/backend/match-reporter/MatchReporter.ts @@ -1,9 +1,11 @@ import { SingleChannelBot } from '../bot/bot-factory' import { MatchReportDecoration, EMPTY_DECORATION } from '../types/MatchReportDecoration' +import { Player } from '../types/Player' +import { Match } from '../types/Match' const DEFAULT_LEADERBOARD_SIZE = 5 -const parseMatchReportDecorations = (config): Array => config +const parseMatchReportDecorations = (config: string): Array => config ? config.split(';').map(configPart => { const splitConfigPart = configPart.split(',') if (splitConfigPart.length !== 4) { @@ -18,7 +20,7 @@ const parseMatchReportDecorations = (config): Array => co }) : [] -const createRankingChangeMessage = (oldRankings, newRankings): string => { +const createRankingChangeMessage = (oldRankings: Player[], newRankings: Player[]): string => { const rankingChanges = oldRankings .map((oldPlayer, index) => ({ name: oldPlayer.name, @@ -38,13 +40,14 @@ const createRankingChangeMessage = (oldRankings, newRankings): string => { .join('\n') } -const hasLeaderboardChanged = (leaderboardSize, oldRankings, newRankings): boolean => { +const hasLeaderboardChanged = +(leaderboardSize: number, oldRankings: Player[], newRankings: Player[]): boolean => { return oldRankings.findIndex((oldPlayer, index) => { return oldPlayer.id !== newRankings[index].id }) < leaderboardSize } -const getDecorationsForTeam = (team, decorations: Array): +const getDecorationsForTeam = (team: Player[], decorations: Array): MatchReportDecoration => { for (const decoration of decorations) { if (team.every(player => player.name === decoration.player1 || @@ -55,7 +58,7 @@ MatchReportDecoration => { return EMPTY_DECORATION } -const createMatchResultMessage = (match, decorations): string => { +const createMatchResultMessage = (match: Match, decorations: MatchReportDecoration[]): string => { const { team1, team2, team1Won, winningTeamRatingChange, losingTeamRatingChange } = match let winningTeam, losingTeam if (team1Won) { @@ -94,9 +97,9 @@ const createMatchResultMessage = (match, decorations): string => { return messageParts.join(' ') } -const ratingComparator = (a, b): number => b.rating - a.rating +const ratingComparator = (a: Player, b: Player): number => b.rating - a.rating -const createPurposeMessage = async (rankings): Promise => { +const createPurposeMessage = async (rankings: Player[]): Promise => { const rankingsText = rankings .map((ranking, i) => `${i + 1}. ${ranking.name} (${ranking.rating})`) .join('\n') @@ -108,7 +111,7 @@ export class MatchReporter { readonly decorations: MatchReportDecoration[] constructor( readonly bot: SingleChannelBot, - matchReportPrefixSuffixConfig, + matchReportPrefixSuffixConfig: string, readonly leaderboardSize = DEFAULT_LEADERBOARD_SIZE ) { try { @@ -118,11 +121,12 @@ export class MatchReporter { } } - async reportMatchOnSlack(match, oldUsers, newUsers): Promise { + async reportMatchOnSlack(match: Match, oldPlayers: Player[], newPlayers: Player[]): + Promise { const matchResultMessage = createMatchResultMessage(match, this.decorations) - const oldRankings = oldUsers.sort(ratingComparator) - const newRankings = newUsers.sort(ratingComparator) + const oldRankings = oldPlayers.sort(ratingComparator) + const newRankings = newPlayers.sort(ratingComparator) const rankingChangeMessage = createRankingChangeMessage(oldRankings, newRankings) await this.bot.postMessage(`${matchResultMessage}\n${rankingChangeMessage}`) diff --git a/backend/package-lock.json b/backend/package-lock.json index d1b76793..eceb66b5 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -560,6 +560,12 @@ "integrity": "sha512-rr+OQyAjxze7GgWrSaJwydHStIhHq2lvY3BOC2Mj7KnzI7XK0Uw1TOOdI9lDoajEbSWLiYgoo4f1R51erQfhPQ==", "dev": true }, + "@types/common-tags": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/@types/common-tags/-/common-tags-1.8.0.tgz", + "integrity": "sha512-htRqZr5qn8EzMelhX/Xmx142z218lLyGaeZ3YR8jlze4TATRU9huKKvuBmAJEW4LCC4pnY1N6JAm6p85fMHjhg==", + "dev": true + }, "@types/connect": { "version": "3.4.33", "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.33.tgz", diff --git a/backend/package.json b/backend/package.json index e5ec9b44..c01068e8 100644 --- a/backend/package.json +++ b/backend/package.json @@ -19,6 +19,7 @@ "slackbots": "^1.2.0" }, "devDependencies": { + "@types/common-tags": "^1.8.0", "@types/express": "^4.17.3", "@types/jest": "^25.1.4", "@types/node": "^13.9.0", diff --git a/backend/rating/rating-calculator.ts b/backend/rating/rating-calculator.ts index 7d977d3f..d8497c8e 100644 --- a/backend/rating/rating-calculator.ts +++ b/backend/rating/rating-calculator.ts @@ -1,6 +1,7 @@ import { RatingChanges } from '../types/RatingChanges' +import { Player } from '../types/Player' -const average = (array): number => { +const average = (array: number[]): number => { const sum = array.reduce((a, b) => a + b) return sum / array.length } @@ -22,7 +23,8 @@ const average = (array): number => { * * @returns {RatingChanges} The rating changes of the teams. */ -export const computeRatingChanges = (winningTeam, losingTeam): RatingChanges => { +export const computeRatingChanges = (winningTeam: Player[], losingTeam: Player[]): +RatingChanges => { const winningAvg = average(winningTeam.map(player => player.rating)) const losingAvg = average(losingTeam.map(player => player.rating)) diff --git a/backend/repositories/MatchRepository.ts b/backend/repositories/MatchRepository.ts index d3471902..9836f7d3 100644 --- a/backend/repositories/MatchRepository.ts +++ b/backend/repositories/MatchRepository.ts @@ -22,7 +22,7 @@ const getFilledTeam = async (playedIds: Array): Promise> = } } -const getRatingChanges = ({ team1, team2 }, team1Won): RatingChanges => { +const getRatingChanges = (team1: Player[], team2: Player[], team1Won: boolean): RatingChanges => { const winningTeam = team1Won ? team1 : team2 const losingTeam = team1Won ? team2 : team1 @@ -31,18 +31,16 @@ const getRatingChanges = ({ team1, team2 }, team1Won): RatingChanges => { const constructMatch = async (gameId: number, matchDescription: MatchDescription): Promise => { - const teams = { - team1: await getFilledTeam(matchDescription.team1), - team2: await getFilledTeam(matchDescription.team2), - } + const team1 = await getFilledTeam(matchDescription.team1) + const team2 = await getFilledTeam(matchDescription.team2) const { winningTeamRatingChange, losingTeamRatingChange, - } = getRatingChanges(teams, matchDescription.team1Won) + } = getRatingChanges(team1, team2, matchDescription.team1Won) const date = new Date() return new Match( - teams.team1, - teams.team2, + team1, + team2, matchDescription.team1Won, date, winningTeamRatingChange, diff --git a/backend/storage/Storage.ts b/backend/storage/Storage.ts index e60ea5f4..b8c97e06 100644 --- a/backend/storage/Storage.ts +++ b/backend/storage/Storage.ts @@ -2,7 +2,7 @@ import * as dbTransactions from './db/db-transactions' import { StorageContext } from './StorageContext' import { User, UserData } from '../types/User' import { MatchWithId, Match } from '../types/Match' -import { Game } from '../types/Game' +import { Game, GameData } from '../types/Game' import { Player } from '../types/Player' export const makeStorageContext = async (): Promise => { @@ -29,7 +29,7 @@ export const getAllUsers = async (): Promise> => { return executeAndCommit(context => context.getAllUsers()) } -export const getUser = async (userId): Promise => { +export const getUser = async (userId: number): Promise => { return executeAndCommit(context => context.getUser(userId)) } @@ -61,7 +61,7 @@ export const getGameByName = async (name: string): Promise => { return executeAndCommit(context => context.getGameByName(name)) } -export const insertGame = async (game): Promise => { +export const insertGame = async (game: GameData): Promise => { return executeAndCommit(context => context.insertGame(game)) } diff --git a/backend/storage/StorageContext.ts b/backend/storage/StorageContext.ts index 74592b64..9b5d154c 100644 --- a/backend/storage/StorageContext.ts +++ b/backend/storage/StorageContext.ts @@ -1,16 +1,18 @@ import * as dbTransformations from './db/db-transformations' import * as dbQueries from './db/db-queries' import * as dbErrors from './db/db-errors' +import { Transaction } from './db/db-transactions' import { ConflictError } from '../errors/ConflictError' import { NotFoundError } from '../errors/NotFoundError' import { User, UserData } from '../types/User' import { MatchWithId, Match } from '../types/Match' import { Game, GameData } from '../types/Game' -import { Player, NULL_PLAYER } from '../types/Player' +import { Player, NULL_PLAYER, PlayerData } from '../types/Player' import { BadRequestError } from '../errors/BadRequestError' +import { QueryResultRow } from 'pg' export class StorageContext { - constructor(private transaction) {} + constructor(private transaction: Transaction) {} async getAllUsers(): Promise> { let rows @@ -60,7 +62,7 @@ export class StorageContext { return dbTransformations.createPlayerFromDbRow(row) } - async getUser(userId): Promise { + async getUser(userId: number): Promise { const query = dbQueries.selectUser const values = [userId] @@ -107,7 +109,7 @@ export class StorageContext { } } - async insertPlayer({ initialRating, userId, gameId }): Promise { + async insertPlayer({ initialRating, userId, gameId }: PlayerData): Promise { try { await this.transaction.executeSingleResultQuery(dbQueries.insertPlayer, [ userId, initialRating, initialRating, gameId, @@ -130,11 +132,7 @@ export class StorageContext { if (!user) { user = await this.insertUser(name) } - await this.insertPlayer({ - initialRating, - userId: user.id, - gameId: game.id, - }) + await this.insertPlayer(new PlayerData(initialRating, user.id, game.id)) } async insertUser(name: string): Promise { @@ -265,7 +263,7 @@ export class StorageContext { } async getGameByName(name: string): Promise { - let row + let row: QueryResultRow try { row = await this.transaction.executeSingleResultQuery(dbQueries.selectGameByName, [name]) } catch (error) { diff --git a/backend/storage/db/db-config.ts b/backend/storage/db/db-config.ts index a7195a52..e9659789 100644 --- a/backend/storage/db/db-config.ts +++ b/backend/storage/db/db-config.ts @@ -1,3 +1,5 @@ +import { PoolConfig } from 'pg' + export const testConfig = { user: 'postgres', host: 'localhost', @@ -6,7 +8,7 @@ export const testConfig = { port: 5432, } -export let productionConfig +export let productionConfig: PoolConfig if (process.env.DATABASE_URL) { productionConfig = { connectionString: process.env.DATABASE_URL, diff --git a/backend/storage/db/db-errors.ts b/backend/storage/db/db-errors.ts index a285f863..a29829bd 100644 --- a/backend/storage/db/db-errors.ts +++ b/backend/storage/db/db-errors.ts @@ -1,9 +1,11 @@ +import { DBError } from '../../types/Database' + export const UNIQUE_VIOLATION_CODE = '23505' export const RAISE_EXCEPTION_CODE = 'P0001' export const CHECK_VIOLATION_CODE = '23514' -export const isUniqueViolation = (error): boolean => error.code == UNIQUE_VIOLATION_CODE +export const isUniqueViolation = (error: DBError): boolean => error.code == UNIQUE_VIOLATION_CODE -export const isRaiseException = (error): boolean => error.code == RAISE_EXCEPTION_CODE +export const isRaiseException = (error: DBError): boolean => error.code == RAISE_EXCEPTION_CODE -export const isCheckViolation = (error): boolean => error.code == CHECK_VIOLATION_CODE +export const isCheckViolation = (error: DBError): boolean => error.code == CHECK_VIOLATION_CODE diff --git a/backend/storage/db/db-transactions.ts b/backend/storage/db/db-transactions.ts index 36113504..a287aa78 100644 --- a/backend/storage/db/db-transactions.ts +++ b/backend/storage/db/db-transactions.ts @@ -1,15 +1,17 @@ -import { Pool, QueryResultRow } from 'pg' +import { Pool, QueryResultRow, PoolClient } from 'pg' import * as dbConfig from './db-config' const pool = new Pool(dbConfig.productionConfig) +type QueryValues = Array + export class Transaction { private active: boolean - constructor(private client) { + constructor(private client: PoolClient) { this.active = true } - async executeQuery(query, values): Promise> { + async executeQuery(query: string, values: QueryValues): Promise> { if (!this.active) { throw new Error('Attempting to execute query on an inactive transaction') } @@ -19,7 +21,8 @@ export class Transaction { return res.rows } - async executeSingleResultQuery(query, values): Promise { + async executeSingleResultQuery(query: string, values: QueryValues): + Promise { const rows = await this.executeQuery(query, values) if (rows.length == 0) { return null diff --git a/backend/storage/db/db-transformations.ts b/backend/storage/db/db-transformations.ts index 9ddf0287..fa18ff98 100644 --- a/backend/storage/db/db-transformations.ts +++ b/backend/storage/db/db-transformations.ts @@ -2,22 +2,42 @@ import { User } from '../../types/User' import { MatchWithId } from '../../types/Match' import { Game } from '../../types/Game' import { Player } from '../../types/Player' +import { QueryResultRow } from 'pg' +import { + isValidMatchRow, + isValidPlayerRow, + isValidGameRow, + isValidUserRow, +} from '../../types/Database' -export const createUserFromDbRow = (userRow): User => ({ - id: Number(userRow.Id), - name: userRow.Name, - active: Boolean(userRow.Active), -}) +export const createUserFromDbRow = (userRow: QueryResultRow): User => { + if (!isValidUserRow(userRow)) { + throw new Error('Invalid row to transform to create User') + } + return new User( + Number(userRow.Id), + userRow.Name, + Boolean(userRow.Active) + ) +} -export const createPlayerFromDbRow = (player): Player => ({ - id: Number(player.Id), - name: player.Name, - rating: Number(player.Rating), - active: Boolean(player.Active), - initialRating: Number(player.InitialRating), -}) +export const createPlayerFromDbRow = (player: QueryResultRow): Player => { + if (!isValidPlayerRow(player)) { + throw new Error('Invalid row to transform to create Player') + } + return new Player( + Number(player.Id), + player.Name, + Number(player.Rating), + Boolean(player.Active), + Number(player.InitialRating), + ) +} -export const createMatchFromDbRow = (matchRow): MatchWithId => { +export const createMatchFromDbRow = (matchRow: QueryResultRow): MatchWithId => { + if (!isValidMatchRow(matchRow)) { + throw new Error('Invalid row to transform to create Match') + } const team1Player2Array = matchRow.Team1Player2Id ? [{ id: Number(matchRow.Team1Player2Id), matchRating: Number(matchRow.Team1Player2Rating), @@ -51,8 +71,13 @@ export const createMatchFromDbRow = (matchRow): MatchWithId => { } } -export const createGameFromDbRow = (gameRow): Game => ({ - id: Number(gameRow.Id), - name: gameRow.Name, - description: gameRow.Description, -}) +export const createGameFromDbRow = (gameRow: QueryResultRow): Game => { + if (!isValidGameRow(gameRow)) { + throw new Error('Invalid row to transform to create Game') + } + return new Game( + Number(gameRow.Id), + gameRow.Name, + gameRow.Description, + ) +} diff --git a/backend/tsconfig.json b/backend/tsconfig.json index e17da983..6bb8b331 100644 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -1,10 +1,12 @@ { - "compilerOptions": { - "module": "commonjs", - "target": "es6", - "sourceMap": true - }, - "exclude": [ - "node_modules" - ] - } \ No newline at end of file + "compilerOptions": { + "module": "commonjs", + "target": "es6", + "sourceMap": true, + "noImplicitAny": true, + "typeRoots": ["./types"] + }, + "exclude": [ + "node_modules" + ], +} diff --git a/backend/types/Database.ts b/backend/types/Database.ts new file mode 100644 index 00000000..0c8f526f --- /dev/null +++ b/backend/types/Database.ts @@ -0,0 +1,77 @@ +import { QueryResultRow } from 'pg' + +export interface PlayerRow { + Id: string; + Name: string; + Rating: string; + Active: string; + InitialRating: string; +} + +export const isValidPlayerRow = (queryRow: QueryResultRow): queryRow is PlayerRow => { + return queryRow.Id && + queryRow.Name && + queryRow.Rating && + queryRow.Active && + queryRow.InitialRating +} + +export interface UserRow { + Id: string; + Name: string; + Active: string; +} + +export const isValidUserRow = (queryRow: QueryResultRow): queryRow is UserRow => { + return queryRow.Id && + queryRow.Name && + queryRow.Active +} + +export interface MatchRow { + Id: string; + Team1Player1Id: string; + Team1Player1Rating: string; + Team1Player2Id: string; + Team1Player2Rating: string; + Team2Player1Id: string; + Team2Player1Rating: string; + Team2Player2Id: string; + Team2Player2Rating: string; + Date: Date; + WinningTeamRatingChange: string; + LosingTeamRatingChange: string; + Team1Won: string; +} + +export const isValidMatchRow = (queryRow: QueryResultRow): queryRow is MatchRow => { + return queryRow.Id && + queryRow.Team1Player1Id && + queryRow.Team1Player1Rating && + queryRow.Team1Player2Id && + queryRow.Team1Player2Rating && + queryRow.Team2Player1Id && + queryRow.Team2Player1Rating && + queryRow.Team2Player2Id && + queryRow.Team2Player2Rating && + queryRow.Date && + queryRow.WinningTeamRatingChange && + queryRow.LosingTeamRatingChange && + queryRow.Team1Won +} + +export interface GameRow { + Id: string; + Name: string; + Description: string; +} + +export const isValidGameRow = (queryRow: QueryResultRow): queryRow is GameRow => { + return queryRow.Id && + queryRow.Name && + queryRow.Description +} + +export interface DBError extends Error { + code: string; +} diff --git a/backend/types/Game.ts b/backend/types/Game.ts index b30b3ff8..de23571e 100644 --- a/backend/types/Game.ts +++ b/backend/types/Game.ts @@ -3,6 +3,10 @@ export interface GameData { readonly description: string; } -export interface Game extends GameData { - readonly id: number; +export class Game { + constructor( + readonly id: number, + readonly name: string, + readonly description: string, + ) {} } diff --git a/backend/types/Player.ts b/backend/types/Player.ts index d8b65f10..093b7154 100644 --- a/backend/types/Player.ts +++ b/backend/types/Player.ts @@ -1,11 +1,19 @@ import { User } from './User' +export class PlayerData { + constructor( + readonly initialRating: number, + readonly userId: number, + readonly gameId: number, + ) {} +} + export class Player extends User { constructor( id: number, name: string, - active: boolean, readonly rating: number, + active: boolean, readonly initialRating: number, ) { super(id, name, active) diff --git a/backend/types/slackbots/index.d.ts b/backend/types/slackbots/index.d.ts new file mode 100644 index 00000000..7c0b1f7c --- /dev/null +++ b/backend/types/slackbots/index.d.ts @@ -0,0 +1,20 @@ +/* eslint-disable */ +declare module 'slackbots' { + interface Config { + token: string + } + interface ApiConfig { + channel: string, + purpose: string + } + interface PostConfig { + as_user: boolean + } + export default class Slackbot { + constructor(config: Config) + on(eventName: string, cb: Function): void + _api(apiName: string, config: ApiConfig): Promise + postTo(channelName: string, message: string, config: PostConfig): Promise + getGroupId(channelName: string): string + } +} From 9ac97baa3dcd61b1bc1f2ed454b2e7e41981f299 Mon Sep 17 00:00:00 2001 From: Roman Vaivod Date: Sun, 26 Apr 2020 01:08:41 +0200 Subject: [PATCH 2/4] gh-133: Fix unit tests --- backend/storage/StorageContext.test.ts | 10 +++++----- backend/tsconfig.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/storage/StorageContext.test.ts b/backend/storage/StorageContext.test.ts index bc8dc488..45451f85 100644 --- a/backend/storage/StorageContext.test.ts +++ b/backend/storage/StorageContext.test.ts @@ -21,11 +21,11 @@ import { ConflictError } from '../errors/ConflictError' import { UNIQUE_VIOLATION_CODE } from './db/db-errors' import * as dbQueries from './db/db-queries' import { MatchWithId } from '../types/Match' +import { Transaction } from './db/db-transactions' +jest.mock('./db/db-transactions') -const TRANSACTION_MOCK = { - executeSingleResultQuery: jest.fn(), - executeQuery: jest.fn(), -} +const MockedTransaction = Transaction as jest.Mock +const TRANSACTION_MOCK = new MockedTransaction() as jest.Mocked describe('StorageContext', () => { let context: StorageContext @@ -144,7 +144,7 @@ describe('StorageContext', () => { }) describe('when Tonda plays foosball', () => { beforeEach(() => { - TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(FOOSBALL_GAME) + TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(FOOSBALL_ROW) TRANSACTION_MOCK.executeQuery.mockResolvedValueOnce([TONDA_PLAYER_ROW]) }) it('resolves with Tonda', async () => { diff --git a/backend/tsconfig.json b/backend/tsconfig.json index 6bb8b331..8b9d75b2 100644 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -4,7 +4,7 @@ "target": "es6", "sourceMap": true, "noImplicitAny": true, - "typeRoots": ["./types"] + "typeRoots": ["./types", "./node_modules/@types"], }, "exclude": [ "node_modules" From 9a5987d63c9aa9c296f55e65393f871272847261 Mon Sep 17 00:00:00 2001 From: Roman Vaivod Date: Sun, 26 Apr 2020 02:25:18 +0200 Subject: [PATCH 3/4] gh-133: Add strict as true --- backend/bot/bot-factory.ts | 4 +++- backend/match-reporter/MatchReporter.ts | 6 ++++-- backend/repositories/GameRepository.test.ts | 4 ++-- backend/repositories/GameRepository.ts | 4 ++-- backend/repositories/MatchRepository.test.ts | 2 +- backend/repositories/MatchRepository.ts | 2 +- backend/storage/Storage.ts | 7 +++++-- backend/storage/StorageContext.test.ts | 11 +++++++---- backend/storage/StorageContext.ts | 19 +++++++++++-------- backend/storage/db/db-transactions.ts | 4 ++-- backend/tests/TestData.ts | 3 ++- backend/tsconfig.json | 2 +- backend/types/Player.ts | 8 +++++++- backend/types/slackbots/index.d.ts | 10 +++++----- 14 files changed, 53 insertions(+), 33 deletions(-) diff --git a/backend/bot/bot-factory.ts b/backend/bot/bot-factory.ts index 8323415e..c660ac51 100644 --- a/backend/bot/bot-factory.ts +++ b/backend/bot/bot-factory.ts @@ -17,10 +17,12 @@ export class SingleChannelBot { } } -export const makeBot = (botToken: string, channelName: string): Promise => { +export const makeBot = +(botToken: string|undefined, channelName: string|undefined): Promise => { return new Promise((resolve, reject): void => { if (!botToken || !channelName) { reject(Error('botToken or channelName missing')) + return } const slackbot = new SlackBot({ diff --git a/backend/match-reporter/MatchReporter.ts b/backend/match-reporter/MatchReporter.ts index 67c8aff4..755cfab5 100644 --- a/backend/match-reporter/MatchReporter.ts +++ b/backend/match-reporter/MatchReporter.ts @@ -5,7 +5,8 @@ import { Match } from '../types/Match' const DEFAULT_LEADERBOARD_SIZE = 5 -const parseMatchReportDecorations = (config: string): Array => config +const parseMatchReportDecorations = +(config: string | undefined): Array => config ? config.split(';').map(configPart => { const splitConfigPart = configPart.split(',') if (splitConfigPart.length !== 4) { @@ -111,13 +112,14 @@ export class MatchReporter { readonly decorations: MatchReportDecoration[] constructor( readonly bot: SingleChannelBot, - matchReportPrefixSuffixConfig: string, + matchReportPrefixSuffixConfig: string | undefined, readonly leaderboardSize = DEFAULT_LEADERBOARD_SIZE ) { try { this.decorations = parseMatchReportDecorations(matchReportPrefixSuffixConfig) } catch (e) { console.warn(`Parsing matchReportPrefixSuffixConfig failed: ${e.message}`) + this.decorations = [] } } diff --git a/backend/repositories/GameRepository.test.ts b/backend/repositories/GameRepository.test.ts index 98ca5d7d..95e0e562 100644 --- a/backend/repositories/GameRepository.test.ts +++ b/backend/repositories/GameRepository.test.ts @@ -1,5 +1,5 @@ import { mocked } from 'ts-jest/utils' -import { FOOSBALL_DATA } from '../tests/TestData' +import { FOOSBALL_DATA, FOOSBALL_GAME } from '../tests/TestData' import { insertGame } from '../storage/Storage' import { addGame } from './GameRepository' @@ -30,7 +30,7 @@ describe('GameRepository', () => { }) describe('when insertGame resolves', () => { beforeEach(() => { - mockedInsertGame.mockResolvedValue(undefined) + mockedInsertGame.mockResolvedValue(FOOSBALL_GAME) }) }) it('stores a valid foosball data via insertGame', async () => { diff --git a/backend/repositories/GameRepository.ts b/backend/repositories/GameRepository.ts index cda9cf01..bed2aca5 100644 --- a/backend/repositories/GameRepository.ts +++ b/backend/repositories/GameRepository.ts @@ -4,8 +4,8 @@ import * as storage from '../storage/Storage' const isValidGameData = (data: unknown): data is GameData => { const gameData = data as GameData - return gameData.name && - gameData.description && + return !!gameData.name && + !!gameData.description && gameData.name.trim() == gameData.name && gameData.name.toLowerCase() == gameData.name && !gameData.name.includes(' ') && diff --git a/backend/repositories/MatchRepository.test.ts b/backend/repositories/MatchRepository.test.ts index a00a5dde..25454965 100644 --- a/backend/repositories/MatchRepository.test.ts +++ b/backend/repositories/MatchRepository.test.ts @@ -41,7 +41,7 @@ describe('MatchRepository', () => { .mockResolvedValueOnce(RADEK_PLAYER) .mockResolvedValueOnce(PETR_PLAYER) mockedGetLatestMatchByGameId.mockResolvedValueOnce(FOOSBALL_MATCH_WITH_ID) - mockedStoreMatch.mockResolvedValueOnce(undefined) + mockedStoreMatch.mockResolvedValueOnce(FOOSBALL_MATCH_WITH_ID) }) describe('called one minute later', () => { lockDate(ONE_MINUTE_AFTER) diff --git a/backend/repositories/MatchRepository.ts b/backend/repositories/MatchRepository.ts index 9836f7d3..c6f279bc 100644 --- a/backend/repositories/MatchRepository.ts +++ b/backend/repositories/MatchRepository.ts @@ -49,7 +49,7 @@ Promise => { ) } -const getElapsedSecondsSinceLatestMatch = async (gameId: number): Promise => { +const getElapsedSecondsSinceLatestMatch = async (gameId: number): Promise => { const latestMatch = await storage.getLatestMatchByGameId(gameId) if (latestMatch == null) { return null diff --git a/backend/storage/Storage.ts b/backend/storage/Storage.ts index b8c97e06..474ee08a 100644 --- a/backend/storage/Storage.ts +++ b/backend/storage/Storage.ts @@ -7,6 +7,9 @@ import { Player } from '../types/Player' export const makeStorageContext = async (): Promise => { const transaction = await dbTransactions.beginTransaction() + if (!transaction) { + throw new Error('Failed to create transaction') + } return new StorageContext(transaction) } @@ -49,7 +52,7 @@ export const getAllMatches = async (): Promise> => { return executeAndCommit(context => context.getAllMatches()) } -export const getLatestMatchByGameId = async (gameId: number): Promise => { +export const getLatestMatchByGameId = async (gameId: number): Promise => { return executeAndCommit(context => context.getLatestMatchByGameId(gameId)) } @@ -61,7 +64,7 @@ export const getGameByName = async (name: string): Promise => { return executeAndCommit(context => context.getGameByName(name)) } -export const insertGame = async (game: GameData): Promise => { +export const insertGame = async (game: GameData): Promise => { return executeAndCommit(context => context.insertGame(game)) } diff --git a/backend/storage/StorageContext.test.ts b/backend/storage/StorageContext.test.ts index 45451f85..6e8099e0 100644 --- a/backend/storage/StorageContext.test.ts +++ b/backend/storage/StorageContext.test.ts @@ -46,7 +46,7 @@ describe('StorageContext', () => { [ 'null', 'null', null, null ], [ 'foosball row', 'foosball game', FOOSBALL_ROW, FOOSBALL_GAME ], ])('when executeSingleResultQuery resolves with %s', (res1Desc, res2Desc, row, result) => { - let foosballGame: Game + let foosballGame: Game|null beforeEach(async () => { TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(row) foosballGame = await context.insertGame(FOOSBALL_DATA) @@ -181,9 +181,12 @@ describe('StorageContext', () => { }) describe('when a new user Tonda is successfully added to foosball game', () => { beforeEach(async () => { + // select Tonda as a user TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(null) + // insert Tonda as a user TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(TONDA_USER_ROW) - TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(undefined) + // insert Tonda as a player + TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(TONDA_PLAYER_ROW) await context.addUserToGame(FOOSBALL_GAME.name, TONDA_PLAYER) }) it('searches game by name', () => { @@ -215,7 +218,7 @@ describe('StorageContext', () => { describe('when an existing player Tonda is successfully added to foosball game', () => { beforeEach(async () => { TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(TONDA_USER_ROW) - TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(undefined) + TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(null) await context.addUserToGame(FOOSBALL_GAME.name, TONDA_PLAYER) }) it('does not insert Tonda as a user', () => { @@ -299,7 +302,7 @@ describe('StorageContext', () => { }) describe('getLatestMatchByGameId', () => { describe('called with foosball id', () => { - let matchWithId: MatchWithId + let matchWithId: MatchWithId|null beforeEach(async () => { TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(FOOSBALL_MATCH_ROW) matchWithId = await context.getLatestMatchByGameId(FOOSBALL_GAME.id) diff --git a/backend/storage/StorageContext.ts b/backend/storage/StorageContext.ts index 9b5d154c..51836fc1 100644 --- a/backend/storage/StorageContext.ts +++ b/backend/storage/StorageContext.ts @@ -9,7 +9,6 @@ import { MatchWithId, Match } from '../types/Match' import { Game, GameData } from '../types/Game' import { Player, NULL_PLAYER, PlayerData } from '../types/Player' import { BadRequestError } from '../errors/BadRequestError' -import { QueryResultRow } from 'pg' export class StorageContext { constructor(private transaction: Transaction) {} @@ -81,7 +80,7 @@ export class StorageContext { return dbTransformations.createUserFromDbRow(row) } - async getUserByName(userName: string): Promise { + async getUserByName(userName: string): Promise { let row try { row = await this.transaction.executeSingleResultQuery(dbQueries.selectUserByName, [userName]) @@ -126,7 +125,7 @@ export class StorageContext { async addUserToGame(gameName: string, { name, initialRating }: UserData): Promise { const game = await this.getGameByName(gameName) if (!game) { - throw new Error(`Unable to find game '${game.name}'`) + throw new Error(`Unable to find game '${gameName}'`) } let user = await this.getUserByName(name) if (!user) { @@ -149,7 +148,9 @@ export class StorageContext { } throw new Error('Unable to add user') } - + if (!row) { + throw new Error(`No user was added with name '${name}'`) + } return dbTransformations.createUserFromDbRow(row) } @@ -194,7 +195,9 @@ export class StorageContext { } throw new Error('Unable to create match') } - + if (!row) { + throw new Error('No match was created') + } return dbTransformations.createMatchFromDbRow(row) } @@ -210,7 +213,7 @@ export class StorageContext { return rows.map(dbTransformations.createMatchFromDbRow) } - async getLatestMatchByGameId(gameId: number): Promise { + async getLatestMatchByGameId(gameId: number): Promise { let row try { row = await this.transaction.executeSingleResultQuery( @@ -233,7 +236,7 @@ export class StorageContext { return rows.map(dbTransformations.createGameFromDbRow) } - async insertGame(game: GameData): Promise { + async insertGame(game: GameData): Promise { let row try { row = await this.transaction.executeSingleResultQuery(dbQueries.insertGame, [ @@ -263,7 +266,7 @@ export class StorageContext { } async getGameByName(name: string): Promise { - let row: QueryResultRow + let row try { row = await this.transaction.executeSingleResultQuery(dbQueries.selectGameByName, [name]) } catch (error) { diff --git a/backend/storage/db/db-transactions.ts b/backend/storage/db/db-transactions.ts index a287aa78..f02c626d 100644 --- a/backend/storage/db/db-transactions.ts +++ b/backend/storage/db/db-transactions.ts @@ -22,7 +22,7 @@ export class Transaction { } async executeSingleResultQuery(query: string, values: QueryValues): - Promise { + Promise { const rows = await this.executeQuery(query, values) if (rows.length == 0) { return null @@ -52,7 +52,7 @@ export class Transaction { } } -export const beginTransaction = async (): Promise => { +export const beginTransaction = async (): Promise => { const client = await pool.connect() try { await client.query('BEGIN') diff --git a/backend/tests/TestData.ts b/backend/tests/TestData.ts index bddb686c..ecbd3e46 100644 --- a/backend/tests/TestData.ts +++ b/backend/tests/TestData.ts @@ -2,6 +2,7 @@ import { MatchWithId, Match } from '../types/Match' import { Player } from '../types/Player' import { MatchDescription } from '../types/MatchDescription' import * as moment from 'moment' +import { UserRow } from '../types/Database' const NOW_MOMENT = moment('2020-03-25 10:00:00') export const NOW = NOW_MOMENT.toDate() @@ -115,7 +116,7 @@ export const FOOSBALL_MATCH: Match = { gameId: 1, } -export const TONDA_USER_ROW = { +export const TONDA_USER_ROW: UserRow = { Id: '4', Name: 'Tonda', Active: 'true', diff --git a/backend/tsconfig.json b/backend/tsconfig.json index 8b9d75b2..c28a6fc7 100644 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -3,7 +3,7 @@ "module": "commonjs", "target": "es6", "sourceMap": true, - "noImplicitAny": true, + "strict": true, "typeRoots": ["./types", "./node_modules/@types"], }, "exclude": [ diff --git a/backend/types/Player.ts b/backend/types/Player.ts index 093b7154..710e96f1 100644 --- a/backend/types/Player.ts +++ b/backend/types/Player.ts @@ -20,4 +20,10 @@ export class Player extends User { } } -export const NULL_PLAYER = new Player(null, null, null, null, null) +export const NULL_PLAYER = { + id: null, + name: null, + rating: null, + active: null, + initialRating: null, +} diff --git a/backend/types/slackbots/index.d.ts b/backend/types/slackbots/index.d.ts index 7c0b1f7c..3122c608 100644 --- a/backend/types/slackbots/index.d.ts +++ b/backend/types/slackbots/index.d.ts @@ -1,15 +1,15 @@ -/* eslint-disable */ declare module 'slackbots' { interface Config { - token: string + token: string; } interface ApiConfig { - channel: string, - purpose: string + channel: string; + purpose: string; } interface PostConfig { - as_user: boolean + as_user: boolean; } + /* eslint-disable import/no-default-export*/ export default class Slackbot { constructor(config: Config) on(eventName: string, cb: Function): void From e613ace2fffc2037fa49570669041a6f4a17325c Mon Sep 17 00:00:00 2001 From: Roman Vaivod Date: Wed, 29 Apr 2020 14:24:15 +0200 Subject: [PATCH 4/4] gh-133: Surround | with spaces --- backend/bot/bot-factory.ts | 2 +- backend/repositories/MatchRepository.ts | 2 +- backend/storage/Storage.ts | 4 ++-- backend/storage/StorageContext.test.ts | 4 ++-- backend/storage/StorageContext.ts | 6 +++--- backend/storage/db/db-transactions.ts | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/backend/bot/bot-factory.ts b/backend/bot/bot-factory.ts index c660ac51..c9e1ebb0 100644 --- a/backend/bot/bot-factory.ts +++ b/backend/bot/bot-factory.ts @@ -18,7 +18,7 @@ export class SingleChannelBot { } export const makeBot = -(botToken: string|undefined, channelName: string|undefined): Promise => { +(botToken: string | undefined, channelName: string | undefined): Promise => { return new Promise((resolve, reject): void => { if (!botToken || !channelName) { reject(Error('botToken or channelName missing')) diff --git a/backend/repositories/MatchRepository.ts b/backend/repositories/MatchRepository.ts index c6f279bc..e9fe6aad 100644 --- a/backend/repositories/MatchRepository.ts +++ b/backend/repositories/MatchRepository.ts @@ -49,7 +49,7 @@ Promise => { ) } -const getElapsedSecondsSinceLatestMatch = async (gameId: number): Promise => { +const getElapsedSecondsSinceLatestMatch = async (gameId: number): Promise => { const latestMatch = await storage.getLatestMatchByGameId(gameId) if (latestMatch == null) { return null diff --git a/backend/storage/Storage.ts b/backend/storage/Storage.ts index 474ee08a..6e62de81 100644 --- a/backend/storage/Storage.ts +++ b/backend/storage/Storage.ts @@ -52,7 +52,7 @@ export const getAllMatches = async (): Promise> => { return executeAndCommit(context => context.getAllMatches()) } -export const getLatestMatchByGameId = async (gameId: number): Promise => { +export const getLatestMatchByGameId = async (gameId: number): Promise => { return executeAndCommit(context => context.getLatestMatchByGameId(gameId)) } @@ -64,7 +64,7 @@ export const getGameByName = async (name: string): Promise => { return executeAndCommit(context => context.getGameByName(name)) } -export const insertGame = async (game: GameData): Promise => { +export const insertGame = async (game: GameData): Promise => { return executeAndCommit(context => context.insertGame(game)) } diff --git a/backend/storage/StorageContext.test.ts b/backend/storage/StorageContext.test.ts index 6e8099e0..dc14bbaf 100644 --- a/backend/storage/StorageContext.test.ts +++ b/backend/storage/StorageContext.test.ts @@ -46,7 +46,7 @@ describe('StorageContext', () => { [ 'null', 'null', null, null ], [ 'foosball row', 'foosball game', FOOSBALL_ROW, FOOSBALL_GAME ], ])('when executeSingleResultQuery resolves with %s', (res1Desc, res2Desc, row, result) => { - let foosballGame: Game|null + let foosballGame: Game | null beforeEach(async () => { TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(row) foosballGame = await context.insertGame(FOOSBALL_DATA) @@ -302,7 +302,7 @@ describe('StorageContext', () => { }) describe('getLatestMatchByGameId', () => { describe('called with foosball id', () => { - let matchWithId: MatchWithId|null + let matchWithId: MatchWithId | null beforeEach(async () => { TRANSACTION_MOCK.executeSingleResultQuery.mockResolvedValueOnce(FOOSBALL_MATCH_ROW) matchWithId = await context.getLatestMatchByGameId(FOOSBALL_GAME.id) diff --git a/backend/storage/StorageContext.ts b/backend/storage/StorageContext.ts index 51836fc1..cc672e52 100644 --- a/backend/storage/StorageContext.ts +++ b/backend/storage/StorageContext.ts @@ -80,7 +80,7 @@ export class StorageContext { return dbTransformations.createUserFromDbRow(row) } - async getUserByName(userName: string): Promise { + async getUserByName(userName: string): Promise { let row try { row = await this.transaction.executeSingleResultQuery(dbQueries.selectUserByName, [userName]) @@ -213,7 +213,7 @@ export class StorageContext { return rows.map(dbTransformations.createMatchFromDbRow) } - async getLatestMatchByGameId(gameId: number): Promise { + async getLatestMatchByGameId(gameId: number): Promise { let row try { row = await this.transaction.executeSingleResultQuery( @@ -236,7 +236,7 @@ export class StorageContext { return rows.map(dbTransformations.createGameFromDbRow) } - async insertGame(game: GameData): Promise { + async insertGame(game: GameData): Promise { let row try { row = await this.transaction.executeSingleResultQuery(dbQueries.insertGame, [ diff --git a/backend/storage/db/db-transactions.ts b/backend/storage/db/db-transactions.ts index f02c626d..f32eb79c 100644 --- a/backend/storage/db/db-transactions.ts +++ b/backend/storage/db/db-transactions.ts @@ -3,7 +3,7 @@ import * as dbConfig from './db-config' const pool = new Pool(dbConfig.productionConfig) -type QueryValues = Array +type QueryValues = Array export class Transaction { private active: boolean @@ -22,7 +22,7 @@ export class Transaction { } async executeSingleResultQuery(query: string, values: QueryValues): - Promise { + Promise { const rows = await this.executeQuery(query, values) if (rows.length == 0) { return null @@ -52,7 +52,7 @@ export class Transaction { } } -export const beginTransaction = async (): Promise => { +export const beginTransaction = async (): Promise => { const client = await pool.connect() try { await client.query('BEGIN')