feat: add multipitch support

[l4u532]

Problem

(See #266)

For a given multi-pitch climb, for each pitch, we want to represent multiple boltsCount and grades, e. g. grade = ["6a", "6c", "5c", "6c"], and boltsCount = [4, 6, 3, 3], and possible other data (like description).

Solution

Introduce type Pitch (with fields parentId, type, number, grade, length, boltsCount, description) as a child to climb, so that climb gets an optional Array Climb.pitches containing each pitch; this has the most flexibility and allows for detailed per-pitch data, plus it wouldn't break any existing implementation

Example JSON Doc Schema

type Climb {
  id: ID!
  uuid: ID!
  name: String!
  ...
  pitches: [Pitch]!  # new pitches array of type Pitch
}

The Type Pitch would contain:

type Pitch {
  uuid: ID!
  parentId: ID!
  number: Int!
  grades: GradeType!
  type: ClimbType!
  length: Int!
  boltsCount: Int
  description: String
}

[l4u532]

@musoke Would you kindly have a look? Thank you =)

[vnugent]

@l4u532 I'll have a look at this issue by this weekend

[musoke]

I had to reread #266 to remind myself of all the nuances discussed previously. This looks good, although I haven't run it myself.

We discussed allowing a pitch to be part of multiple routes, to account for variations, etc. Does this PR allow for that to be added in future? The number and parent fields assume only one parent route.

[musoke]

What is number? Is this to track the order of the pitches?

[musoke]

Looks like it from the rest of the changes.

[l4u532]

number is to track the sequence of pitches. It is also only required field besides id and parentId. The reasoning: the correct sequence of information is essential for multi-pitches. A user might only add the pitches' grades; but it is prime that she does so in correct order, else the value of information is almost zero.

Since this seems to be non-intuitive, should I rename this field for more clarity? E. g. pitchNumber? (Suggestions welcome)

[vnugent]

We should use something else besides number to avoid conflict with number in Typescript. pitchNumber sounds good. At first I thought pitch number isn't needed since we can get them from the array index. But in some case it may not be easy to get the index. Glad you thought of it.

[l4u532]

I had to reread #266 to remind myself of all the nuances discussed previously. This looks good, although I haven't run it myself.

We discussed allowing a pitch to be part of multiple routes, to account for variations, etc. Does this PR allow for that to be added in future? The number and parent fields assume only one parent route.

See #350 (comment) for my reply.

[vnugent]

@l4u532 Thank you very much for this PR, and I apologize for my delayed response. I left my feedback inline.

[vnugent]

We should use something else besides number to avoid conflict with number in Typescript. pitchNumber sounds good. At first I thought pitch number isn't needed since we can get them from the array index. But in some case it may not be easy to get the index. Glad you thought of it.

[vnugent]

we should remove _id: true since we will be providing our own UUID. _id: true means Mongo will auto-generate _id for us in ObjectID format (not UUID).

[l4u532]

@vnugent I have incorporated all changes and would be happy to get your re-review, especially on lines 77-81, since I am not sure if I have interpreted your suggestions correctly: https://github.com/OpenBeta/openbeta-graphql/pull/350/files#diff-9b4028942ce1d106899ff51138cc6ee7cc75ea36110257394c51424643261967R77-R81

[vnugent]

Thanks for the updating the code. I think we're very close now. I left some comments inline.

[l4u532]

Thanks for the updating the code. I think we're very close now. I left some comments inline.

Incorporated the requested changes.

FYI, there is a phenomenon which I do not yet quite understand. I assume its harmless. When updating pitches, "id" is added - additionally to "_id":

    pitches: [
        {
            id: 'a011077d-58bd-4edd-9d9c-e5f3a4520add', # <-- duplicate, when updating the pitch
            parentId: UUID("97737f2ddac94ecda7645490ec1c40a0"),
            pitchNumber: 1,
            grades: {
                ewbank: '19'
            },
            type: {
                sport: false,
                alpine: true
            },
            length: 20,
            boltsCount: 6,
            description: 'Updated first pitch description',
            _id: UUID("a011077d58bd4edd9d9ce5f3a4520add") # <-- original
        },

[vnugent]

FYI, there is a phenomenon which I do not yet quite understand. I assume its harmless. When updating pitches, "id" is added - additionally to "_id"

As mentioned the spread operator ...pitch in the db update code introduces the id field.

Once you fix that, we're good to merge. I think we may need to create an additional create/update/delete pitch API to make it easier to work with pitch data.

[l4u532]

FYI, there is a phenomenon which I do not yet quite understand. I assume its harmless. When updating pitches, "id" is added - additionally to "_id"

As mentioned the spread operator ...pitch in the db update code introduces the id field.

Once you fix that, we're good to merge. I think we may need to create an additional create/update/delete pitch API to make it easier to work with pitch data.

Sorry for the belated fix, I was on vacation. I am kindly asking you to review this hopefully final fix :) @vnugent

[vnugent]

I'm traveling at the moment. Will get back to you by next week.