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.

Sign up for GitHub

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

Jump to bottom

Fix/user type #1830

Merged
merged 27 commits into from
Feb 14, 2024
Merged

Fix/user type #1830

merged 27 commits into from
Feb 14, 2024

Conversation

AVtheking
Copy link

What kind of change does this PR introduce?

Fix the usage of userType ,created a new appUserProfile model

Fixes #1711

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

Ye

@github-actions GitHub Actions
Copy link

Our Pull Request Approval Process

We have these basic policies to make the approval process smoother for our volunteer team.

Testing Your Code

Please make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:

  1. The overall code coverage drops below the target threshold of the repository
  2. Any file in the pull request has code coverage levels below the repository threshold
  3. Merge conflicts

The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

@palisadoes palisadoes requested review from SiddheshKukade, xoldd and EshaanAgg and removed request for SiddheshKukade, xoldd and EshaanAgg February 10, 2024 13:04
@palisadoes
Copy link
Contributor

Please fix the failing application tests

@AVtheking
Copy link
Author

image
sir my test is failing because of this but this package is already initialized in app.ts file but still this is throwing error ?

@palisadoes
Copy link
Contributor

Could it be due to any of the most recent merged PRs since your most recent commits?

@AVtheking
Copy link
Author

Maybe but this issue is being faced by other contributors too

@palisadoes
Copy link
Contributor

It's something else. You may have discovered a bug:

File updatePluginStatus.spec.ts was edited in this PR last month

File createMessageChat.spec.ts was edited in this PR 10 months ago

You can verify the mutations this here:

You can verify the non-test equivalents here:

File adminCheck.spec.ts was edited 4 months ago

@palisadoes
Copy link
Contributor

  1. Are you sure others are seeing this issue? More recent PRs are not failing because of this.
  2. Have you tried merging with the latest develop?

@AVtheking
Copy link
Author

Yes I have asked in the slack channel and there also one mate said he is facing same issue

@Cioppolo14
Copy link
Contributor

@EshaanAgg @SiddheshKukade Can you review this PR?

@AVtheking
Squashed commit of the following:
4c39066 
commit e50baf9
Merge: 3fd0675 65989b1
Author: ANKIT VARSHNEY <132201033+AVtheking@users.noreply.github.com>
Date:   Tue Feb 13 23:34:06 2024 +0530

    Merge branch 'PalisadoesFoundation:develop' into develop

commit 65989b1
Author: Paras Awasthi <121304240+i-m-Paras@users.noreply.github.com>
Date:   Tue Feb 13 18:21:56 2024 +0530

    removeAdvertisement.ts test updated to 100% coverage (PalisadoesFoundation#1835)

    * removeAdvertisement.ts test updated to 100% coverage

    * failing tests updated

    * Update package.json

    * Update package.json

    * Update package-lock.json

    * Update package.json

    * Update package.json

    * Update package.json

    * Update package.json

    * Update package-lock.json back to normal

    * Update package-lock.json

    * Update package-lock.json

    * Update package-lock.json

    * Update package-lock.json

commit cc21e65
Author: Anvita Mahajan <78889572+Anvita0305@users.noreply.github.com>
Date:   Tue Feb 13 18:21:20 2024 +0530

    Added tests for file src/resolvers/Mutation/createPost.ts (PalisadoesFoundation#1833)

    * Modified tests

    * some minor changes

    * Fixed Errors

    * 100% code covered

    * Fixed Linting errors

commit f95b71f
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Feb 12 10:12:01 2024 -0800

    chore(deps): bump @types/validator from 13.11.8 to 13.11.9 (PalisadoesFoundation#1845)

    Bumps [@types/validator](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/validator) from 13.11.8 to 13.11.9.
    - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
    - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/validator)

    ---
    updated-dependencies:
    - dependency-name: "@types/validator"
      dependency-type: direct:development
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit a6a5758
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Feb 12 09:51:43 2024 -0800

    chore(deps): bump @graphql-codegen/typescript from 4.0.1 to 4.0.4 (PalisadoesFoundation#1846)

    Bumps [@graphql-codegen/typescript](https://github.com/dotansimha/graphql-code-generator/tree/HEAD/packages/plugins/typescript/typescript) from 4.0.1 to 4.0.4.
    - [Release notes](https://github.com/dotansimha/graphql-code-generator/releases)
    - [Changelog](https://github.com/dotansimha/graphql-code-generator/blob/master/packages/plugins/typescript/typescript/CHANGELOG.md)
    - [Commits](https://github.com/dotansimha/graphql-code-generator/commits/@graphql-codegen/typescript@4.0.4/packages/plugins/typescript/typescript)

    ---
    updated-dependencies:
    - dependency-name: "@graphql-codegen/typescript"
      dependency-type: direct:development
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit b738582
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Feb 12 09:49:35 2024 -0800

    chore(deps): bump lint-staged from 15.2.1 to 15.2.2 (PalisadoesFoundation#1844)

    Bumps [lint-staged](https://github.com/okonet/lint-staged) from 15.2.1 to 15.2.2.
    - [Release notes](https://github.com/okonet/lint-staged/releases)
    - [Changelog](https://github.com/lint-staged/lint-staged/blob/master/CHANGELOG.md)
    - [Commits](lint-staged/lint-staged@v15.2.1...v15.2.2)

    ---
    updated-dependencies:
    - dependency-name: lint-staged
      dependency-type: direct:development
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 56cb29f
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Feb 12 09:49:16 2024 -0800

    chore(deps): bump nodemailer from 6.9.8 to 6.9.9 (PalisadoesFoundation#1843)

    Bumps [nodemailer](https://github.com/nodemailer/nodemailer) from 6.9.8 to 6.9.9.
    - [Release notes](https://github.com/nodemailer/nodemailer/releases)
    - [Changelog](https://github.com/nodemailer/nodemailer/blob/master/CHANGELOG.md)
    - [Commits](nodemailer/nodemailer@v6.9.8...v6.9.9)

    ---
    updated-dependencies:
    - dependency-name: nodemailer
      dependency-type: direct:production
      update-type: version-update:semver-patch
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit b445770
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Feb 12 09:48:58 2024 -0800

    chore(deps): bump @typescript-eslint/eslint-plugin from 6.20.0 to 6.21.0 (PalisadoesFoundation#1842)

    Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 6.20.0 to 6.21.0.
    - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
    - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
    - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v6.21.0/packages/eslint-plugin)

    ---
    updated-dependencies:
    - dependency-name: "@typescript-eslint/eslint-plugin"
      dependency-type: direct:development
      update-type: version-update:semver-minor
    ...

    Signed-off-by: dependabot[bot] <support@github.com>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 125853a
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 23:20:44 2024 +0530

    Update pull_changes.yml

commit d069d40
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 22:57:38 2024 +0530

    Update Advertisement.ts

commit 5fe0ae8
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 22:44:37 2024 +0530

    Update pull_changes.yml

commit 26e0d10
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 22:40:11 2024 +0530

    Update push.yml

commit 60d4b4b
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 22:18:30 2024 +0530

    Update push.yml

commit 07f7042
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 22:01:00 2024 +0530

    Update Advertisement.ts

commit 12ccdb6
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 21:29:14 2024 +0530

    Update Advertisement.ts

commit 4e6113d
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 21:26:51 2024 +0530

    Update push.yml

commit 73b68f0
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:57:18 2024 +0530

    Update Advertisement.ts

commit 70701ba
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:54:11 2024 +0530

    Update push.yml

commit 57f4d91
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:30:17 2024 +0530

    Update Advertisement.ts

commit e1bbff6
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:25:45 2024 +0530

    Update pull_changes.yml

commit 332c429
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:06:36 2024 +0530

    Update pull_changes.yml

commit d7450c4
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 20:06:01 2024 +0530

    Update push.yml

commit 7b9a88f
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 19:50:17 2024 +0530

    Update push.yml

commit 94d2d18
Author: TASNEEM KOUSHAR <imatasneemkoushar@gmail.com>
Date:   Sun Feb 11 19:39:08 2024 +0530

    Create pull_changes.yml

commit 031adf9
Author: Peter Harrison <16875803+palisadoes@users.noreply.github.com>
Date:   Sat Feb 10 19:27:48 2024 -0800

    Restore the automated-docs if statement

commit 3fd0675
Merge: ad9a412 fa10711
Author: ANKIT VARSHNEY <132201033+AVtheking@users.noreply.github.com>
Date:   Fri Feb 9 00:59:59 2024 +0530

    Merge branch 'PalisadoesFoundation:develop' into develop
@palisadoes palisadoes removed their request for review February 13, 2024 20:26
@EshaanAgg
Copy link
Contributor

The diff for this PR is 7k lines added and 3k lines removed. This is not reviewable. Please figure out a way to reduce the diff or break it in smaller parts.

@palisadoes
Copy link
Contributor

@EshaanAgg

Given that this feature touches so many aspects of the apps it's going to be complicated

  • Can you suggest ways to simplify it?

There are other considerations:

  1. It's not being merged into develop, so we could:
    1. Create similar branches in the other repos
    2. Create issues to make the new branches compatible with this one
    3. Create issues to make all new features on develop address to this branch

Next week we expect a lot of activity that is like to channel productively. We could take advantage of the GSoC evaluation period to get this long overdue feature properly implemented instead of having to manage lots of one one line PRs.

I think it's worth taking the chance. Do you in think so in that context?

@EshaanAgg
Copy link
Contributor

Sounds good.

  • If we want smaller PRs, we can ask the contributors to migrate only a certain number of resolvers (~10) in PR so that the same remains reviewable.
  • If that seems too much work, we can take the risk and rely on manual and feature testing, as highlighted in your approach. Once the same is stable in all branches, we can merge into the develop branch in all the related repositories.

@palisadoes
Copy link
Contributor

I'm going to merge it. Having some resolvers working the new way while others work the old way will cause hardship.

@palisadoes
Copy link
Contributor

@AVtheking

  1. Do you feel sufficiently comfortable with the web front end to apply the necessary changes to work with this PR?
  2. How would you propose to split up the Talawa-Admin PRs for this to make them easier to review?

@AVtheking
Copy link
Author

@palisadoes sir Although I have not worked with graphql client before in the frontend but have experience in working with the frontend so I can make it through

We can raise pr according to feature wise of the application ,in that way it can be easily reviewed

@palisadoes
Copy link
Contributor

  1. Can you suggest a series of PRs that we'd need?
  2. I want to know how we could split the workload.

@palisadoes palisadoes merged commit 9321a1c into PalisadoesFoundation:develop-userTypeFix Feb 14, 2024
7 of 11 checks passed
@palisadoes
Copy link
Contributor

@AVtheking

  1. I just realized that we created this issue in Admin too.
    1. Admin: Fix the usage of userType talawa-admin#1445
  2. I think it'll be best to monitor the develop branch and create issues to correctly merge new develop PRs into develop-userTypeFix. You can create issues when necessary. Could you do that?
  3. You'll also need to assist in troubleshooting the Admin PR that will certainly need assistance. New issues could be created for that independently by you for that too.

What do you think?

@AVtheking
Copy link
Author

I didn't get the second point
Yes I am ready to assist the admin PRs

@AVtheking
Copy link
Author

I would tell series of PRs in about half an hour sir

@palisadoes palisadoes mentioned this pull request Feb 14, 2024
Closed
@palisadoes
Copy link
Contributor

  1. Now that I found the other issue, please coordinate any work with the assignee of that issue
    1. Admin: Fix the usage of userType talawa-admin#1445
  2. We have many PRs in the queue against the API develop branch that will need to be merged into the develop-userTypeFix branch as soon as these PRs are merged into develop .
    1. This will need to be done until we finally merge develop-userTypeFix into develop when the Talawa-Admin work on develop-userTypeFix is complete.
    2. When the Talawa-Admin work on develop-userTypeFix is complete, that branch will have to be merged into develop too.

Does that clarify the path to take?

@AVtheking
Copy link
Author

Ok sir I understood

This was referenced Feb 14, 2024
Closed
Closed
Closed
Closed
@palisadoes palisadoes mentioned this pull request Feb 27, 2024
Closed
@AVtheking AVtheking mentioned this pull request Mar 5, 2024
Merged
@palisadoes palisadoes mentioned this pull request Mar 11, 2024
Closed
@Atharva-Kanherkar
Copy link

@AVtheking Can i contact you on slack?

@AVtheking
Copy link
Author

@Atharva-Kanherkar yeah sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EshaanAgg EshaanAgg Awaiting requested review from EshaanAgg

@SiddheshKukade SiddheshKukade Awaiting requested review from SiddheshKukade

Assignees
No one assigned
Labels
Ready 4 PR Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AVtheking @palisadoes @Cioppolo14 @EshaanAgg @Atharva-Kanherkar