Skip to content

Commit

Permalink
Merge pull request #218 from sandboxnu/AP/unsubscribe-after-3-notifs
Browse files Browse the repository at this point in the history
Ap/unsubscribe after 3 notifs
  • Loading branch information
ananyaspatil authored Oct 5, 2024
2 parents 33e48e4 + ecc8013 commit 6cd914e
Show file tree
Hide file tree
Showing 12 changed files with 884 additions and 89 deletions.
31 changes: 30 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,31 @@ on:
branches:
- master
jobs:
unit:
name: Unit Tests
runs-on: ubuntu-latest
timeout-minutes: 10
env:
TWILIO_ACCOUNT_SID: AC_dummy_value_so_tests_dont_error_out
TWILIO_AUTH_TOKEN: 123
elasticURL: "http://localhost:9200"
NODE_COVERALLS_DEBUG: 1
steps:
- uses: actions/checkout@v2
- name: install node
uses: actions/setup-node@v2
with:
node-version: "16"
- uses: bahmutov/npm-install@v1

- run: yarn unittest --coverage
- name: Coveralls
uses: coverallsapp/github-action@master
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
flag-name: Unit tests
parallel: true

tests:
name: Tests
runs-on: ubuntu-latest
Expand Down Expand Up @@ -37,6 +62,10 @@ jobs:
node-version: "16"
- uses: bahmutov/npm-install@v1

- run: yarn db:migrate

- run: yarn db:refresh

- run: yarn test --coverage --detectOpenHandles
- name: Coveralls
uses: coverallsapp/github-action@master
Expand Down Expand Up @@ -149,7 +178,7 @@ jobs:
# - run: 'curl ''http://localhost:4000'' -X POST -H ''content-type: application/json'' --data ''{ "query": "query { search(termId: \"202250\", query: \"fundies\") { nodes { ... on ClassOccurrence { name subject classId } } } }" }'''

coverage:
needs: [end_to_end, tests]
needs: [end_to_end, tests, unit]
name: Sends Coveralls coverage
runs-on: ubuntu-latest
env:
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
"about:tsc": "//// Runs TSC (because of our config, this will NOT compile the files, just typecheck them)",
"tsc": "tsc",
"about:test": "//// Runs a basic suite of unit and integration tests",
"test": "yarn jest --verbose --testPathIgnorePatterns='(seq|reg|git).[jt]s'",
"test": "yarn jest --verbose --testPathIgnorePatterns='(seq|reg|git|unit).[jt]s'",
"about:dbtest": "//// Runs tests interacting with our database. Must have the Docker containers running. This won't touch live data - it will create and teardown a temporary database.",
"dbtest": "jest -i --projects tests/database --verbose",
"about:unittest": "//// Runs unit tests. Does not need db, elasticsearch, spun up. Does not need the Docker containers to be running.",
"unittest": "jest -i --projects tests/unit --verbose",
"about:build_backend": "//// Compiles this project",
"build_backend": "rm -rf dist && mkdir -p dist && babel --extensions '.js,.ts' . -d dist/ --copy-files --ignore node_modules --ignore .git --include-dotfiles && rm -rf dist/.git",
"about:build": "//// Compiles this project, surpressing output",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- AlterTable
ALTER TABLE "followed_courses" ADD COLUMN "notif_count" INTEGER NOT NULL DEFAULT 0;

-- AlterTable
ALTER TABLE "followed_sections" ADD COLUMN "notif_count" INTEGER NOT NULL DEFAULT 0;
2 changes: 2 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ model Subject {
model FollowedCourse {
courseHash String @map("course_hash")
userId Int @map("user_id")
notifCount Int @default(0) @map("notif_count")
course Course @relation(fields: [courseHash], references: [id], onDelete: Cascade)
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
Expand All @@ -101,6 +102,7 @@ model FollowedCourse {
model FollowedSection {
sectionHash String @map("section_hash")
userId Int @map("user_id")
notifCount Int @default(0) @map("notif_count")
section Section @relation(fields: [sectionHash], references: [id], onDelete: Cascade)
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
Expand Down
48 changes: 43 additions & 5 deletions services/notifyer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* See the license file in the root folder for details.
*/

import { User } from "@prisma/client";
import { User, FollowedSection } from "@prisma/client";

Check warning on line 6 in services/notifyer.ts

View workflow job for this annotation

GitHub Actions / Lint & Type checks

'FollowedSection' is defined but never used. Allowed unused vars must match /^_/u
import prisma from "./prisma";
import twilioNotifyer from "../twilio/notifs";
import macros from "../utils/macros";
import {
Expand Down Expand Up @@ -45,9 +46,20 @@ export async function sendNotifications(
return;
} else {
const courseNotifPromises: Promise<void>[] = notificationInfo.updatedCourses
.map((course) => {
const courseMessage = generateCourseMessage(course);
.map(async (course) => {
const users = courseHashToUsers[course.courseHash] ?? [];

await prisma.followedCourse.updateMany({
where: {
courseHash: course.courseHash,
userId: { in: users.map((u) => u.id) },
},
data: {
notifCount: { increment: 1 },
},
});

const courseMessage = generateCourseMessage(course);
return users.map((user) => {
return twilioNotifyer.sendNotificationText(
user.phoneNumber,
Expand All @@ -59,9 +71,21 @@ export async function sendNotifications(

const sectionNotifPromises: Promise<void>[] =
notificationInfo.updatedSections
.map((section) => {
const sectionMessage = generateSectionMessage(section);
.map(async (section) => {
const users = sectionHashToUsers[section.sectionHash] ?? [];

//increment notifCount of this section's entries in followedSection
await prisma.followedSection.updateMany({
where: {
sectionHash: section.sectionHash,
userId: { in: users.map((u) => u.id) },
},
data: {
notifCount: { increment: 1 },
},
});

const sectionMessage = generateSectionMessage(section);
return users.map((user) => {
return twilioNotifyer.sendNotificationText(
user.phoneNumber,
Expand All @@ -71,6 +95,20 @@ export async function sendNotifications(
})
.reduce((acc, val) => acc.concat(val), []);

//delete any entries in followedCourse w/ notifCount >= 3
await prisma.followedCourse.deleteMany({
where: {
notifCount: { gt: 2 },
},
});

//delete any entries in followedSection w/ notifCount >= 3
await prisma.followedSection.deleteMany({
where: {
notifCount: { gt: 2 },
},
});

await Promise.all([...courseNotifPromises, ...sectionNotifPromises]).then(
() => {
macros.log("Notifications sent from notifyer!");
Expand Down
5 changes: 4 additions & 1 deletion services/updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ class Updater {
this.SECTION_MODEL
);

//Filter out courseHash & sectionHash if they have too high notifsSent

await sendNotifications(
notificationInfo,
courseHashToUsers,
Expand Down Expand Up @@ -518,7 +520,8 @@ class Updater {
const columnName = `${modelName}_hash`;
const pluralName = `${modelName}s`;
const dbResults = (await prisma.$queryRawUnsafe(
`SELECT ${columnName}, JSON_AGG(JSON_BUILD_OBJECT('id', id, 'phoneNumber', phone_number)) FROM followed_${pluralName} JOIN users on users.id = followed_${pluralName}.user_id GROUP BY ${columnName}`
//test edit: edited this select cmd to filter out any followed_modelName w/ notifsSent greater than 2
`SELECT ${columnName}, JSON_AGG(JSON_BUILD_OBJECT('id', id, 'phoneNumber', phone_number)) FROM followed_${pluralName} JOIN users on users.id = followed_${pluralName}.user_id WHERE notif_count < 3 GROUP BY ${columnName}`
)) as Record<string, any>[];

return Object.assign(
Expand Down
62 changes: 0 additions & 62 deletions tests/database/search.test.seq.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import searcher from "../../services/searcher";
import prisma from "../../services/prisma";
import { LeafQuery, ParsedQuery } from "../../types/searchTypes";

beforeAll(async () => {
searcher.subjects = {};
Expand All @@ -23,67 +22,6 @@ describe("searcher", () => {
});
});

//Unit tests for the parseQuery function
describe("parseQuery", () => {
it("query with no phrases", () => {
const retQueries: ParsedQuery = searcher.parseQuery(
"this is a query with no phrases"
);
expect(retQueries.phraseQ.length).toEqual(0); //no phrase queries
expect(retQueries.fieldQ).not.toEqual(null);
const fieldQuery: LeafQuery = retQueries.fieldQ;

expect(fieldQuery).toEqual({
multi_match: {
query: "this is a query with no phrases",
type: "most_fields",
fields: searcher.getFields(),
},
});
});

it("query with just a phrase", () => {
const retQueries: ParsedQuery = searcher.parseQuery('"this is a phrase"');

expect(retQueries.phraseQ.length).toEqual(1);
expect(retQueries.fieldQ).toEqual(null);

const phraseQuery: LeafQuery = retQueries.phraseQ[0];

expect(phraseQuery).toEqual({
multi_match: {
query: "this is a phrase",
type: "phrase",
fields: searcher.getFields(),
},
});
});

it("query with a phrase and other text", () => {
const retQueries: ParsedQuery = searcher.parseQuery('text "phrase" text');
expect(retQueries.phraseQ.length).toEqual(1);
expect(retQueries.fieldQ).not.toEqual(null);

const phraseQuery: LeafQuery = retQueries.phraseQ[0];
const fieldQuery: LeafQuery = retQueries.fieldQ;

expect(phraseQuery).toEqual({
multi_match: {
query: "phrase",
type: "phrase",
fields: searcher.getFields(),
},
});

expect(fieldQuery).toEqual({
multi_match: {
query: "text text",
type: "most_fields",
fields: searcher.getFields(),
},
});
});
});
// TODO: create an association between cols in elasticCourseSerializer and here
describe("generateQuery", () => {
it("generates match_all when no query", () => {
Expand Down
Loading

0 comments on commit 6cd914e

Please sign in to comment.