Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow user scope to view old events #153

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions packages/ilmomasiina-backend/src/models/event.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import moment from "moment";
import {
DataTypes,
HasManyAddAssociationMixin,
Expand Down Expand Up @@ -211,20 +210,6 @@ export default function setupEventModel(sequelize: Sequelize) {
[Op.and]: {
// are not drafts,
draft: false,
// and either:
[Op.or]: {
// closed less than a week ago
registrationEndDate: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
// or happened less than a week ago
date: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
endDate: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
},
Comment on lines -214 to -227
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll still want a hard limit for regular users

},
},
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { FastifyReply, FastifyRequest } from "fastify";
import { NotFound } from "http-errors";
import moment from "moment";
import { Op } from "sequelize";

import type {
Expand Down Expand Up @@ -32,7 +33,25 @@ export const basicEventInfoCached = createCache({
async get(eventSlug: EventSlug) {
// First query general event information
const event = await Event.scope("user").findOne({
where: { slug: eventSlug },
where: {
slug: eventSlug,
// are not drafts,
draft: false,
// and either:
[Op.or]: {
// closed less than a week ago
registrationEndDate: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
// or happened less than a week ago
date: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
endDate: {
[Op.gt]: moment().subtract(7, "days").toDate(),
},
},
},
attributes: eventGetEventAttrs,
include: [
{
Expand Down
20 changes: 16 additions & 4 deletions packages/ilmomasiina-backend/src/routes/events/getEventsList.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
import { col, fn, Order } from "sequelize";
import { col, fn, Op, Order, WhereOptions } from "sequelize";

import type { AdminEventListResponse, EventListQuery, UserEventListResponse } from "@tietokilta/ilmomasiina-models";
import { adminEventListEventAttrs, eventListEventAttrs } from "@tietokilta/ilmomasiina-models/dist/attrs/event";
Expand All @@ -24,8 +24,20 @@ function eventOrder(): Order {
export const eventsListForUserCached = createCache({
maxAgeMs: 1000,
maxPendingAgeMs: 2000,
async get(category?: string) {
const where = category ? { category } : {};
async get(options: { category?: string; since?: Date }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work with the current caching system, which compares the function argument with Object.is (using a Map)

const { category, since } = options;
const filters: WhereOptions = {};
if (category) {
filters.category = category;
}
if (since) {
filters.date = {
[Op.gt]: since,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Op.gte would probably be more exact

};
}
const where = {
[Op.and]: filters,
};

const events = await Event.scope("user").findAll({
attributes: eventListEventAttrs,
Expand Down Expand Up @@ -68,7 +80,7 @@ export async function getEventsListForUser(
throw new InitialSetupNeeded("Initial setup of Ilmomasiina is needed.");
}

const res = await eventsListForUserCached(request.query.category);
const res = await eventsListForUserCached({ category: request.query.category, since: request.query.since });
reply.status(200);
return res as StringifyApi<typeof res>;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/ilmomasiina-models/src/schema/eventList/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ export const eventListQuery = Type.Object({
description: "If set, only events with the provided category are included.",
}),
),
since: Type.Optional(
Type.Date({
description: "If set, only events starting after this date are included.",
}),
),
Comment on lines +41 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type.Date is a typebox extension, it doesn't actually parse the dates. You'll need to use Type.String with format: "date-time" and parse yourself (like we do in e.g. updateEvent)

});

/** Query parameters applicable to the public event list API. */
Expand Down
Loading