sei-ec-remote / project-2-issues

1 stars 0 forks source link

trying to use Document.prototype.id on my subdocument DESTROY route #143

Closed shaialoni closed 2 years ago

shaialoni commented 2 years ago

What's the problem you're trying to solve?

Trying to use Document.prototype.id to identify and remove a subdocument from my model's array of subs.

Post any code you think might be relevant (one fenced block per file)

// Parent Schema
const mongoose = require('./connection')
const eventSchema = require('./event.js')
const { Schema, model } = mongoose

const calSchema = new Schema({
    name: String,
    description: String,
    owner: {
        type: Schema.Types.ObjectId,
        required: true,
        ref: 'User'   
    },
    events: [eventSchema],
}, {
    timestamps: true
})

const PersonalCal = model('PersonalCal', calSchema)

module.exports = PersonalCal
// Child schema
const mongoose = require('./connection')
const calSchema = require('./personalCal')
const { Schema, model } = mongoose

const eventSchema = new Schema({
    title: String,
    date: String,
    hdate: String,
    category: String,
    hebrew: String,
    memo: String,
    yomtov: {
        type: Boolean,
        required: false
    },
    calId: String,
    owner: {
        type: Schema.Types.ObjectId,
        ref: 'User',
        required: false   
    },
}, {
    timestamps: true
})

const Event = model('Event', eventSchema)

module.exports = Event
//DESTROY route - delete event from personal calendar
// test code - currently document.id() function is not working. WHY???
router.delete('/show/:calId/:eventId', (req, res) => {
    const {eventId} = req.params
    const {calId} = req.params
    PersonalCal.findById(calId) 
        .then(calendar => {
            console.log(' lets check it out', calendar.events.id)
            const event = calendar.events.id(eventId)
            console.log(eventId, calId)
            event.remove()
            return event.save()
        })
        .then(event => {
            res.redirect(`/personal/${calId}`)
        })
        .catch(err => console.log(err))
})

If you see an error message, post it here. If you don't, what unexpected behavior are you seeing?

From terminal: TypeError: calendar.events.id is not a function at /Users/shaialoni/sei/projects/project_2/controller/personalCal_routes.js:227:43 at processTicksAndRejections (node:internal/process/task_queues:96:5)

What is your best guess as to the source of the problem?

None. This code exactly works in the fruit app......

What things have you already tried to solve the problem?

Tried reading up in doc to see if some weird condition applies to my code...

Paste a link to your repository here

https://github.com/shaialoni/SEIR_Project_2

kestler01 commented 2 years ago

const event = calendar.events.id(eventId)

isn't correct, and since it's plural ( and a sub document) i assume it's an array - try using an array method there to get the one you want.

also, we should save the parent document, not the subdocument

shaialoni commented 2 years ago

calendar is the parent document events is where the array of subdocs live

I switched the .save to the parent This is the code form the fruit app fruit is my calendar, comments is my events

Fruit.findById(fruitId) // single fruit doc - many comments
    // find this comment by it's ID
        .then(fruit => {
            console.log('fruit', fruit.comments.id)
            const comment = fruit.comments.id(commId)
            console.log('i am the comment', comment)
            console.log('commid & fruitId', commId, fruitId)
            // remove the comment
            comment.remove()

            // i've changed the comments field by 1
            return fruit.save()
shaialoni commented 2 years ago

This is how I did it before, but you suggested I use the mongoose models when dealing with subdocuments, so I am trying to do it the right way

//working code
router.delete('/show/:calId/:eventId', (req, res) => {
    const {eventId} = req.params
    const {calId} = req.params
    PersonalCal.findOne({_id: calId})
        .then(cal => {
            console.log(cal.events[0]._id)
            cal.events.forEach((item, i) => {
                if (item._id == eventId) {
                    cal.events.splice(i, 1)
                    cal.save()
                    res.redirect(`/personal/${calId}`)
                }
            })
        })
        .catch(err => console.log(err))
})
kestler01 commented 2 years ago

Totally understandable. We we changed your model from an array of objects to an array of subdocuments we still have that array there which we have to deal with.

If I remember correctly the major reason we wanted to go to sub docs was to follow best practices and allow mongoose to assign our Id's so that their all unique and pass type validations. Plus we should never allow a user to mutate the id ( and in many cases shouldn't let them see it at all)

This was also the source of that weird bug you got when you had 2 events with the same id and it broke your server requests

shaialoni commented 2 years ago

You are absolutely right. So I fixed that, I use the Model.create() to create new events and that takes care of the ID. I am doing something a bit weird for edit, but thats a different conversation :) Is the method I used before (labeled working code) acceptable? I understand that just because it works doesn't mean we can use it... I don't have a heavy workload for today, so if at some point you have some free time, I think all I would need is a few minutes of showing you what I have to finish my work

kestler01 commented 2 years ago

yes that is acceptable - i think i would have used find since you're only looking for one of the resource but that's personal preference how is it working in app ?

shaialoni commented 2 years ago

As expected. This is what I was using before I switched from just objects in arrays to the subdocuments. I was dealing with it a lot yesterday too, I initially tried to use Model .(different delete methods) but none of those worked. They would erase the data, but the title stayed in some kind of a memory, so since I display bu title they would still show up even though they are erased.....? At this point I am not sure anymore how any of these mongoose commands are supposed to work, so I am just happy I have a working product :/

kestler01 commented 2 years ago

totally understandable - mongoose commands can be tricky, especially when the share names with other generic functions and methods but work differently. The answer for that is unfortunately ' check the documentation ' < Ominous Wizard Voice> Glad you have the route working - closing the issue