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

[Feat] Notification sent with Login to Github button when not authenticated #80

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Prince-Mendiratta
Copy link
Contributor

@Prince-Mendiratta Prince-Mendiratta commented Mar 26, 2023

Issue(s)

$ Fixes #47

Check for the following commands if the notification displayed has the "Github Login" button along with the message prompt to login.

  • /github me
  • /github subscribe
  • /github Username/RepositoryName subscribe
  • /github Username/RepositoryName unsubscribe
  • /github issue
  • /github issues
  • /github search

Proposed changes (including videos or screenshots)

Firstly, to keep code DRY and avoid repetition of code, I've created a separate method to create the Login ActionBlock. This method is reused in all places to create the action block. All the required commands will now directly show the Login to Github button, avoiding extra steps to authenticate.

Second important change is that in the Assign and Share Issues -> /github issues command, it will not work without authorization first. I added this because this was mentioned in the expected results by @Nabhag8848 in the linked issue.

Results

https://drive.google.com/file/d/1Cqe1dwObEMZ9fv0X_69O-a27icAhEjVR/view?usp=sharing

cc @samad-yar-khan @Nabhag8848

@Prince-Mendiratta
Copy link
Contributor Author

gentle bump @samad-yar-khan

@samad-yar-khan
Copy link
Collaborator

@Prince-Mendiratta

  • /github issues works without auth aswell
  • /github search would also need this

@Prince-Mendiratta
Copy link
Contributor Author

@samad-yar-khan

Reverted changes for /github issues.
/github search already had the functionality, I missed out on that in the description. Added now.

Signed-off-by: Prince Mendiratta <[email protected]>
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.

[FEAT] Send Notification with Login ActionBlock When User Not logged In
2 participants