GitHub App: link to app from import page #606
Conversation
There was a problem hiding this comment.
I'm in doubt about this. The button is specific for GitHub while we do support other providers here as well. Is this a temporary workaround while we are in the transition?
What would be the next steps after the transition? Will we remove the "Refresh repositories"? Will we keep both buttons?
| href="https://github.com/apps/{{ GITHUB_APP_NAME }}/installations/new/" | ||
| target="_blank"> | ||
| <i class="fa-brands fa-github icon"></i> | ||
| {% trans "Install GitHub App" %} |
There was a problem hiding this comment.
| {% trans "Install GitHub App" %} | |
| {% trans "Install GitHub App (beta)" %} |
There was a problem hiding this comment.
We should be consistent with how we are describing this, we don't really describe this as a beta anywhere else except maybe the migration page that blocks the last few steps.
I'm find labeling it as beta, but we should do it everywhere.
There was a problem hiding this comment.
Regarding content, this doesn't communicate that this step might be required every single project creation. If I had already connected/installed the GHA, I would assume I didn't need to click this button ever again.
It should specifically call out that the user needs to update the GHA permissions.
There was a problem hiding this comment.
I think this is the only real thing stopping this PR from going out. We can make more improvements in a following UI.
| {% trans "Install GitHub App" %} | |
| {% trans "Update GitHub App permissions" %} |
|
The GH option is only displayed for users that are using the GH app. |
This is my question as well. I think it's confusing to have two buttons to solve a potential single issue and we should only have a single button. Also, pointed out in the other PR, our UI should depend on how often the user might hit this. If users will need to update the permissions every project creation, we should make the UI for updating permissions a bit more obvious. |
I understand the user will need to install the GHA on every project creation if they haven't selected the option to install GHA in all the projects at once and instead selected specific projects. One doubt that I have is: will those projects that where our GHA is not installed appear on the listing? If not, that's a problem... Can we list all the projects there (even those where our GHA is not installed) and make the installation of the GHA part of the "Add project" workflow? |
|
I think I'm picturing similar issues with UX there. It seems like we'll need the GHA permission step to be more mandatory. For current users, we'll guide them through adding permissions to repositories during migration. But going forward, each new repository and every new user using GHA will probably have to more frequently update the GHA permissions. I'm still using OAuth on CircleCI, how do they handle this UX with GHA? |
Yeah. I see the same here. The workflow to "Add a project" will be different for users using GHA (without granting access to all repositories) because they will first need to grant our GHA access to the repository and then select the repository from the list. Unfortunately, we have no way to list the repository first because as we don't have access to that repository, we don't even know it exists 😞 This first step (granting access to the RTD GHA using the external GitHub page) is what I'm saying it has to be more integrated into the the "Add project" workflow. That said, I'm fine if this integration is not perfect yet and it's just some copy with a link to the external GitHub page; but we need to communicate that required first step clearly. Note that we were already confused about this and we are the ones creating this feature. So, I would expect users to be pretty confused here as well 😄 . We need to guide them. |
|
What is the status here? Are we moving forward with this with the GH App deploy? |
|
This wasn't ready for yesterday's deploy. We need to figure it out how to implement the workflow mentioned in my previous comment: #606 (comment) |
|
We could decide if we want to continue with this PR, but I was thinking about jumping into adding some more major UI here next. I agree so far that the button at the bottom probably won't be enough for some cases. It feels like we'll need some UI before the user can search:
Can we detect any of the above scenarios or do we need to guess at all these? |
|
@stsewd You know the most here, can we detect any of the above scenarios? |
We can know this, but we are not saving this information in the model.
Don't think we can know this directly, but GH will prompt the user to "request" approval to grant access to additional projects. |
That might be a great addition for later. We'll need to see exactly what we give users for an intermediate UI first, and then if it makes more sense to improve the repository selection UI |
| href="https://github.com/apps/{{ GITHUB_APP_NAME }}/installations/new/" | ||
| target="_blank"> | ||
| <i class="fa-brands fa-github icon"></i> | ||
| {% trans "Install GitHub App" %} |
There was a problem hiding this comment.
I think this is the only real thing stopping this PR from going out. We can make more improvements in a following UI.
| {% trans "Install GitHub App" %} | |
| {% trans "Update GitHub App permissions" %} |
Reverts #606 user_has_github_app_account wasn't implemented in .org, reverting until it's implemented.
The code template tag and setting will be added in readthedocs/readthedocs.org#12217 once we agree if we are okay with this change.