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.

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

Fix #564: Improve error checking logic #667

Merged

Conversation

relausen
Copy link
Contributor

@relausen relausen commented Nov 3, 2024

Describe what this PR does / why we need it

Improves the error handling and user notifications when the user does not want to use Projects v2 by default, i.e., when not setting the default_to_projects_v2 config option or setting it to false.

In the current code users see an unexpected error message about missing gh token scopes in two different scenarios:

  1. They did not enable default_to_projects_v2 and they do not have the read:project or the project scopes in their gh token
  2. They do have either the read:project or the project scope in their gh token but have not set the default_to_projects_v2 or have set it to false.

In other words: They will always receive an error message if default_to_projects_v2 is disabled.

They should only recive an error message if they enable default_to_projects_v2 and do not have one of the necessary scopes in their token.

The PR changes the logic in the check such that it only checks for token scopes if the user actually enables default_to_projects_v2.

I believe this change makes the "how to suppress the error" part in the corresponding section in the FAQ unnecessary, but I'm not sure. Please let me know if this assumption is correct, and I will update the README as part of this PR.

Does this pull request fix one issue?

Fixes #564

Describe how you did it

Changed a conditional checking both default_to_projects_v2 and token set conditions (using and) to instead check for default_to_projects_v2 first. If that is not set to true then we don't need to check for scopes.

Describe how to verify it

  1. Do not set default_to_projects_v2 or set it to false in the Octo config. Ensure that the project and read:project scopes are not set. Load the plugin and confirm that no error message is shown.
  2. Like 1, but set one of the scopes.
  3. Set default_to_projects_v2 to true. Ensure that the project and read:project scopes are not set. Load the plugin and confirm that an error message is shown.
  4. Set default_to_projects_v2 to true. Ensure that the project and read:project scopes are set. Load the plugin and confirm that an error message is not shown.

Special notes for reviews

Checklist

  • Passing tests and linting standards
  • Documentation updates in README.md and doc/octo.txt

@relausen relausen changed the title Fix 564: Improve error checking logic Fix #564: Improve error checking logic Nov 3, 2024
@wd60622
Copy link
Collaborator

wd60622 commented Nov 3, 2024

Hi @relausen
Thanks for making a PR! I will take a look when I can

@pwntester
Copy link
Owner

Thanks for the PR, LGTM 👍🏼

@pwntester pwntester merged commit 3f640db into pwntester:master Nov 4, 2024
2 checks passed
@TheLeoP
Copy link
Contributor

TheLeoP commented Nov 5, 2024

Could an option to silence the Using Projects V2 message be added? It's noisy to always show it on startup

TheLeoP added a commit to TheLeoP/nvim-config that referenced this pull request Nov 5, 2024
@relausen
Copy link
Contributor Author

relausen commented Nov 5, 2024

Could an option to silence the Using Projects V2 message be added? It's noisy to always show it on startup

It was an honest oversight from my side that it wasn't removed before I submitted the PR. I added it for testing, and forgot to delete it. I think you should submit a PR for this :-)

@TheLeoP TheLeoP mentioned this pull request Nov 5, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot request projects v2, missing scope 'read:project'
4 participants