Upgrade package mongoose from 5.13.22 to 8.0.4

[palisadoes]

Rationale

1. This was previously attempted by the automated dependabot job but the PR tests failed.
2. This issue has been created to fix the issue as there may be multiple dependency requirements that need updating

This is a major revision upgrade and many files may need to be updated to the new syntax, functions, methods and classes

Task

Upgrade mongoose from 5.13.22 to 8.0.4.

Background Failing PRs

• chore(deps): bump mongoose from 5.13.22 to 8.0.4 #1689
• https://github.com/PalisadoesFoundation/talawa-api/actions/runs/7526222330/job/20483956849?pr=1689

Release Note Details

Release notes

Sourced from mongoose's releases.

8.0.4 / 2024-01-08

• fix(update): set CastError path to full path if casting update fails #14161 #14114
• fix: cast error when there is an elemMatch in the and clause #14171 tosaka-n
• fix: allow defining index on base model that applies to all discriminators #14176 peplin
• fix(model): deep clone bulkWrite() updateOne arguments to avoid mutating documents in update #14197 #14164
• fix(populate): handle deselecting _id with array of fields in populate() #14242 #14231
• types(model+query): use stricter typings for updateX(), replaceOne(),deleteX() Model functions #14228 #14204
• types: fix return types for findByIdAndDelete overrides #14196 #14190
• types(schema): add missing omit() method #14235 amitbeck
• types(model): add missing strict property to bulkWrite() top level options #14239
• docs(compatibility): add note that Mongoose 5.13 is fully compatible with MongoDB server 5 #14230 #14149
• docs: add shared schemas guide #14211
• docs: update TLS/SSL guide for Mongoose v8 - MongoDB v6 driver deprecations #14170 andylwelch
• docs: update findOneAndUpdate tutorial to use includeResultMetadata #14208 #14207
• docs: clarify disabling _id on subdocs #14195 #14194

8.0.3 / 2023-12-07

• fix(schema): avoid creating unnecessary clone of schematype in nested array so nested document arrays use correct constructor #14128 #14101
• docs(connections): add example of registering connection event handlers #14150
• docs(populate): add example of using refPath and ref functions #14133 #13834
• types: handle using BigInt global class in schema definitions #14160 #14147
• types: make findOneAndDelete() without options return result doc, not ModifyResult #14153 #14130
• types(model): add no-generic override for insertMany() with options #14152 #13999
• types: add missing Type for applyDefaults #14159 jaypea

8.0.2 / 2023-11-28

• fix(populate): set populated docs in correct order when populating virtual underneath doc array with justOne #14105
• fix(populate): fix curPath to update appropriately #14099 #14098 csy1204
• types: make property names show up in intellisense for UpdateQuery #14123 #14090
• types(document): correct return type for doc.deleteOne() re: Mongoose 8 breaking change #14110 #14081
• types: correct types for when includeResultMetadata: true is set #14078
• types(models): allow specifying timestamps as inline option for bulkWrite() operations #14112 #14072
• docs: fix rendering of 7.x server compatibility #14086 laupow
• docs(source/api): fix "index.js" -> "mongoose.js" rename #14125
• docs(README): update breaking change version #14126

8.0.1 / 2023-11-15

• fix: retain key order with aliases when creating indexes with alias #14042 meabed
• fix: handle nonexistent collection with diffIndexes #14029 #14010
• types(model+query): correctly remove count from TypeScript types to reflect removal of runtime support #14076 #14067 #14062
• types: correct this parameter for methods and statics #14028 #14027 ruxxzebre
• types(model+query): unpack arrays in distinct return type #14047 #14026
• types: add missing Types.UUID typings #14023 #13103 k725
• docs: add mongoose 8 to mongodb server compatibility guide #14064
• docs: fix typo in queries.md #14065 MuhibAhmed

... (truncated)

Changelog

Sourced from mongoose's changelog.

8.0.4 / 2024-01-08

• fix(update): set CastError path to full path if casting update fails #14161 #14114
• fix: cast error when there is an elemMatch in the and clause #14171 tosaka-n
• fix: allow defining index on base model that applies to all discriminators #14176 peplin
• fix(model): deep clone bulkWrite() updateOne arguments to avoid mutating documents in update #14197 #14164
• fix(populate): handle deselecting _id with array of fields in populate() #14242 #14231
• types(model+query): use stricter typings for updateX(), replaceOne(),deleteX() Model functions #14228 #14204
• types: fix return types for findByIdAndDelete overrides #14196 #14190
• types(schema): add missing omit() method #14235 amitbeck
• types(model): add missing strict property to bulkWrite() top level options #14239
• docs(compatibility): add note that Mongoose 5.13 is fully compatible with MongoDB server 5 #14230 #14149
• docs: add shared schemas guide #14211
• docs: update TLS/SSL guide for Mongoose v8 - MongoDB v6 driver deprecations #14170 andylwelch
• docs: update findOneAndUpdate tutorial to use includeResultMetadata #14208 #14207
• docs: clarify disabling _id on subdocs #14195 #14194

7.6.8 / 2024-01-08

• perf(schema): remove unnecessary lookahead in numeric subpath check
• fix(discriminator): handle reusing schema with embedded discriminators defined using Schema.prototype.discriminator #14202 #14162
• fix(ChangeStream): avoid suppressing errors in closed change stream #14206 #14177

6.12.5 / 2024-01-03

• perf(schema): remove unnecessary lookahead in numeric subpath check
• fix(document): allow setting nested path to null #14226
• fix(document): avoid flattening dotted paths in mixed path underneath nested path #14198 #14178
• fix: add ignoreAtomics option to isModified() for better backwards compatibility with Mongoose 5 #14213

6.12.4 / 2023-12-27

• fix: upgrade mongodb driver -> 4.17.2
• fix(document): avoid treating nested projection as inclusive when applying defaults #14173 #14115
• fix: account for null values when assigning isNew property #14172 #13883

8.0.3 / 2023-12-07

• fix(schema): avoid creating unnecessary clone of schematype in nested array so nested document arrays use correct constructor #14128 #14101
• docs(connections): add example of registering connection event handlers #14150
• docs(populate): add example of using refPath and ref functions #14133 #13834
• types: handle using BigInt global class in schema definitions #14160 #14147
• types: make findOneAndDelete() without options return result doc, not ModifyResult #14153 #14130
• types(model): add no-generic override for insertMany() with options #14152 #13999
• types: add missing Type for applyDefaults #14159 jaypea

7.6.7 / 2023-12-06

• fix: avoid minimizing single nested subdocs if they are required #14151 #14058
• fix(populate): allow deselecting discriminator key when populating #14155 #3230

... (truncated)

Commits

• ec4191e chore: release 8.0.4
• 3bc4482 Merge branch '7.x'
• 2c2377d Merge pull request #14242 from Automattic/vkarpov15/gh-14231
• ac9af5b docs: add unnecessary lookahead fix to changelog
• 6ffb123 chore: release 7.6.8
• 9b759bc fix(populate): handle deselecting _id with array of fields in populate()
• 889a41d Merge pull request #14240 from Automattic/vkarpov15/gh-14236
• 3d4274e Merge pull request #14239 from Automattic/vkarpov15/gh-14234
• 62223bf Merge pull request #14235 from amitbeck/types/schema_omit
• 3b2a138 fix(query): avoid double-calling query transform() with findOne()
• Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

[palisadoes]

@xoldyckk Your comments on this would be valuable.

[Veer0x1]

I would like to work on it.

[NayOoLwin5]

Hi @palisadoes, I do have experience in migration from 5 to 7 in my current projects. Can you please assign this to me? I would like to work on it knowing this is migration to 8. Thanks.

[Cioppolo14]

@Veer0x1 Please make sure you include any comments or advice @xoldyckk has on this.

[Veer0x1]

@palisadoes @xoldyckk On upgrading the mongoose package to 8.0.4 , it could return a null document also, and the resolver types generated by codegen would not allow it.
For example:
here updateLanguage should of type User according to Resolver but the query is returning that it could of type User | null

And this is just not with single Resolver, there are tons of other. So how could I fix it.
The approaches I could think of now is:

1. We store the return document to variable and while returning that document explicitly mention that isn't null

to like this

2. Returning the document as the required Interface

3. We can do error handling when document is null like this. But for this we have to also write tests which would be alot of work.

If you have any better approach then please suggest.
One one more thing mongoose will stop support for mongoose 5 after 1st march 2024

[EshaanAgg]

@Veer0x1 Thanks for pointing this out. I feel that the best would be the culmination of two approaches:

• In all of the field level resolvers, that is, resolvers for fields of models like User, Organization, Event etc. we should be using approach 1 as the same just get rid of the null type, as we can be assured that these resolvers would always be called by GraphQL such that their _id exists.
• In all of the mutations and queries, we should use the error-handling approach. Most of them already have the if not exists for objects, so that same shouldn't be too much of a burden to refactor.

I would like to wait for the opinions of @xoldyckk before proceeding.

[palisadoes]

@Veer0x1

Of the three options, which one do you think is the best solution in the long term for stability and maintainability, and why?

[xoldd]

@Veer0x1

Notice the usage of bang operator ! after CreateEventPayload, it signifies whether the type is nullable or not, graphql codegen generates type definitions according to that.

type Mutation {
  createEvent(input: CreateEventInput!): CreateEventPayload!
}

If the field type is non nullable in graphql schema and database returns a null, you throw a GraphqlError with appropriate error message and extensions or you just return null, former method is preferable because it allows more precise error communication to the client.

If the field type is non nullable in graphql schema and an unexpected error happens in the resolver, you either catch the error within the resolver and throw a GraphqlError with appropriate error message and extensions or you let a parent error handling layer take care of it, former method is preferable because it allows more precise error communication to the client.

type Mutation {
  createEvent(input: CreateEventInput!): CreateEventPayload
}

If the field type is nullable in graphql schema and database returns a null, you return the null.

If the field type is nullable in graphql schema and an unexpected error happens in the resolver, you catch the error within the resolver and throw a GraphqlError with appropriate error message and extensions.

[Veer0x1]

Thanks @xoldyckk for your advice, I'll go as you suggested.

[Veer0x1]

@palisadoes this changes would be a lot of changes for a single PR, if there isn't any issue in reviewing then I can do all this changes in a single PR.

[palisadoes]

1. Do it as a single PR. We can't partially upgrade Mongoose
2. Make sure the test cases pass and that the tests are valid
3. We need to get this completed before any GSoC announcements. Fingers crossed that we get through.

[DMills27]

@xoldyckk Can you go into more details regarding the extensions for the error messages? I'd opt for the same route that you're suggesting but I'm a bit worried its effect on the tests as you've previously suggested.

[xoldd]

https://the-guild.dev/graphql/yoga-server/docs/features/error-masking#error-codes-and-other-extensions

extensions is a field on GraphqlError class that lets use provide additional information about the error.

[Veer0x1]

@xoldyckk wouldn't it be better to handle error using our custom errors classes like this

[xoldd]

the custom error class is built upon the standard javascript Error class instead of the GraphqlError class, so these error throws are not perceived by the graphql engine as having a special relation to the resolver they're thrown in, it is anti-graphql error handling, ideally the graphql engine would map those errors to paths corresponding to those resolvers in the graph where they occurred

the localization aspect of error messages (different languages) is useless unless those error messages are displayed to the end user, developers of the UI applications write code in english language as is the standard across the world, same goes for the consuming the developer centric error messages

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[Cioppolo14]

@Veer0x1 Are you still working on this?

[Veer0x1]

Yes I'm working on it, fixing some failing tests.

[palisadoes]

@Veer0x1

When do you think you'll be able to raise a PR? The end of support date is in a few weeks

[Veer0x1]

@Veer0x1

When do
you think you'll be able to raise a PR? The end of support date is in a few weeks

I'll try my best to raise PR by this week, most part of upgradation is done, just fixing some tests.

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[gautam-divyanshu]

@meetulr @palisadoes This problem happens where populate("creator","-password") as well; it is not limited to the agenda category. It is occurring as a result of a breaking change in Mongoose (https://mongoosejs.com/docs/migrating_to_6.html#strictpopulate).

[gautam-divyanshu]

@palisadoes @meetulr @xoldyckk should I remove populate() where there is no path in schema, I didn't find any resources to handle this situation on internet

[xoldd]

@gautam-divyanshu populate() is used to resolve foreign key relation between documents and include the resolved documents in the parent document. So to resolver the creator of a post along with the post you'd populate on the creatorId field:-

const postWithCreator = await Post.findOne({
  where: {
    _id: equals(postId)
  },
})
.populate("creatorId"))

This would be the shape of resolved postWithUser object:-

{
  _id: somePostId,
  creatorId: {
    _id: someUserId,
    ...other user fields 
  },
  ...other post fields
}

organization is not a legit path to populate, but mongoose doesn't report type errors when non existent fields are populated, otherwise this error would be caught by typescript compiler instead of running the test for that file.

The correct way to resolve foreign key relations between different collections in graphql is explained here:- #1585 (comment)

[gautam-divyanshu]

@xoldyckk okay, Thanks for help.

[AVtheking]

@gautam-divyanshu I think instead of organization there should be organizationId because it is used as foreign key there .

[Atharva-Kanherkar]

@palisadoes @xoldyckk organisation isn't a path in schema, so there's nothing to populate().and there is no option available to set strictpopulate to false. I have reviewed their official documentation and found no solution, nor have I found any answers on Google

Its Agenda Category, @Atharva-Kanherkar worked on it, it's part of his ongoing issue #1588 . You could confirm with him if some corrections are required here. Btw there seems to be a mistake in the currentUserIsOrgAdmin too.

I think because i was thinking it would work, maybe i did not check this, Any update on this now @gautam-divyanshu I am happy to help!

[gautam-divyanshu]

@gautam-divyanshu I think instead of organization there should be organizationId because it is used as foreign key there .

Yes, I've already changed that.

[gautam-divyanshu]

ny update on this n

I have changed creator to creatorId and organisation to organisationId, and test cases are passing, Thanks btw.

[gautam-divyanshu]

@palisadoes @meetulr @xoldyckk I just fetched updates from remote repo to my local repo, and got this error while running tests

[palisadoes]

We are frequently occupied with managing other PRs and issues so you'll need to take the initiative to solve some of these roadblocks.

It will be very difficult to keep up with the changes unless:

1. there is a code freeze or
2. you decide to make your changes against the develop-fixUserType branch which hopefully will be merged back into develop this month.

I'd prefer not to freeze the code in develop as it's so active and we have good momentum in adding new features. You have also said that this won't be necessary, but synchronizing your updates seems to be challenging.

The other concern is that if we have a freeze and there is a snag, all other work will be indefinitely on hold.

We merge into develop almost daily. We merge into develop-fixUserType about every other week. The next one should be this weekend after which time we can do a freeze without impacting develop work.

I think this approach will be more rewarding by increasing the likelihood of success.

[gautam-divyanshu]

@palisadoes @Cioppolo14
I can make changes againt the develop-fixUserType branch, which would be more better approach.
If feasible, please freeze this branch; I will do my best to submit a PR by this weekend.

[palisadoes]

The new functionality with a user's userType being assigned per organization needs to be maintained.

Read the details of this issue to understand what you expect

• API: Fix the usage of userType #1711

[gautam-divyanshu]

@palisadoes Do I need to make any changes to that branch?, or can I simply merge my branch into this branch and resolve the merge conflicts?

[gautam-divyanshu]

@palisadoes And this branch is not showing up in my forked repository; do I need to refork it?

[palisadoes]

Refetch from the upstream

[gautam-divyanshu]

@palisadoes @Cioppolo14 please review this pr
#1972

[gautam-divyanshu]

@palisadoes Is this correct way?
Created two Prs

[gautam-divyanshu]

@palisadoes @Cioppolo14 I have created one pr (i.e. #1977 ) for minor change to work around github actions and other pr (i.e. #1978 ) which is related to actual issue

Please review these pr