OpenBeta / openbeta-graphql

The open source rock climbing API
https://openbeta.io
GNU Affero General Public License v3.0
42 stars 32 forks source link

Ticks should store the date in millis #427

Closed Vichy97 closed 2 days ago

Vichy97 commented 5 days ago

When sorting ticks, we can't ensure they are in the correct order for a day. Two climbs on the same day are not necessarily ordered in the ticked-order because we only record the date as a string and don't have any additional time stamp. This isn't a huge deal and most users probably won't care, but millis are the standard way of recording time and how the rest of the api records timestamps.

julianlam commented 5 days ago

Having the end user specify time in addition to date may be little excessive.

I'd recommend making it optional. It's likely if the end user doesn't enter a time, they wouldn't care about the order being wrong.

Vichy97 commented 3 days ago

We don't need the user to specify time. If the date is stored in millis, time can be implicitly captured.

CocoisBuggy commented 2 days ago

I'm gonna close the issue on this repo, but I'd strongly encourage you to open it on the open-tacos repo, which is where the date restriction is happening. Allowing users to optionally select a time should be quite straightforward.

Here is my reasoning, feel free to point out something I have missed

/**
 * Tick Schema
 *
 * The tick schema defines how ticks are stored and serialized in the mongo database.
 * see the TickTypes.ts file for the typescript interface that defines types as they
 * are used within the application. Getting documents from this schema should kick out
 * TickType objects.
 */
export const TickSchema = new Schema<TickType>({
  name: { type: Schema.Types.String, required: true, index: true },
  notes: { type: Schema.Types.String, required: false },
  climbId: { type: Schema.Types.String, required: true, index: true },
  userId: { type: Schema.Types.String, required: true, index: true },
  style: { type: Schema.Types.String, required: true, default: '' },
  attemptType: { type: Schema.Types.String, required: true, index: true, default: '' },
  dateClimbed: { type: Schema.Types.Date },
  grade: { type: Schema.Types.String, required: true, index: true },
  // Bear in mind that these enum types must be kept in sync with the TickSource enum
  source: { type: Schema.Types.String, enum: ['MP', 'OB'] as TickSource[], required: true, index: true }
})

The data type already supports date input to ms level precision. more here.

the current tick creation method takes the data and doesn't truncate the date

async addTick (tick: TickInput): Promise<TickType> {
    return await this.tickModel.create({ ...tick })
  }

This is possibly its own issue... but that aside

I did double check just for sanity's sake and I could put regular untruncated dates into the GQL mutation such that they would appear in the DB as they should be.