Allow hiding issue fork branches

fjgarlin → closed merge request !206

We currently have a "status" text field in the "drupalorg_issue_fork_branches" table. Currently, it's populated when a merge request is closed, and we can see hidden branches, as shown at the top of this issue.

I suggest using that property to show/hide the branch, but it'll be triggered from within the issue page, instead of reading from a merge_request webhook event.

The links to show / hide the branch could be something like this:

Clicking on these will write to the database directly, with no effect to any existing MRs status.

If we are happy with this approach I can work on it, otherwise happy to hear other options.

Let’s keep hiding/showing on MR on close/re-open with the webhook. Usually, closing the MR is a clear indication that it isn’t relevant anymore. (Core doing manual merges is an exception, but I think that’s okay.)

And, let’s add the UI to update the same drupalorg_issue_fork_branches.status field with the UI.

For UX, I could imagine people not knowing that hide/show will hide/show for everyone. With the old files UI, the Display checkbox and needing to save the issue is a bit heavier of an interaction, so you’re more likely to know it's updating the overall issue. We don't have a place to do that below “Add new comment” now, and I don’t think we should add one.

Adding links to the table as proposed should be best. Let’s keep the label simply hide/show, and add something like title="Hides branch for everyone"; we can do something different if we get reports that its confusing.

Ready to test:
- Styling issue for bluecheese 📌 Style new link to show and hide branches within an issue Needs review
- URL to test: https://fjgarlin-drupal.dev.devdrupal.org/project/config_notify/issues/3...
- MR: https://git.drupalcode.org/project/drupalorg/-/merge_requests/207/diffs (remember the "@todo" before deploying).

Test cases:
- If users don't have fork access, the link will not be shown, and even if they were to find it and visit it, it will check their fork membership and bail out. They will need to request access and then the links will show.
- If users do have fork access, the links will show, and then you can show or hide the branch as needed.

This dev instance is not totally in sync with the gitlab dev instance, so it's kinda difficult to test the full journey without some html crafting.
Workarounds to test when you don't have fork access:
- If you try in this instance "Get push access", it will just fail. You can add the class "have-push-access" to the body of the page and the links will show.
- Then, when you click on the link, by default it will tell you that you do not have access (which is correct). To bypass this and test the actual toggling as if you would have access, you can edit the link "href" attribute and add "?bypass=1" (this bypass will be removed before deploying). The toggling should work in this case.

As per @drumm feedback. I am checking only if the logged in user has permissions to edit issues. Same test URL and MRs as 17.

Thanks for the feedback. Ready for review again.

All feedback was addressed.

Ready to test:
- Styling issue for bluecheese 📌 Style new link to show and hide branches within an issue Needs review
- URL to test: https://fjgarlin-drupal.dev.devdrupal.org/project/config_notify/issues/3...
- MR: https://git.drupalcode.org/project/drupalorg/-/merge_requests/207/diffs

This has been deployed!

I think we should consider reverting this issue until and rework it so that it is clearer you are hiding the branch for all users. We've put the show/hide in the same location as the old show/hide button for files that used to collapse or open the files for the current user. This new button hides or shows for all users. You used to have to make a comment to hide files for all users. It's also near to the show commands button which also doesn't make a change to the issue for all users.

At the very least there should be a confirmation that you want to do this for all users.

Also I think that any change like this by a user should result in a comment. It'd be super annoying if a user went and hide MRs on 100s of issues and we did not know who it was.

I added the automatic comment when "hide" or "show" are clicked.
MR here: https://git.drupalcode.org/project/drupalorg/-/merge_requests/212/diffs
Test URL: https://fjgarlin-drupal.dev.devdrupal.org/project/config_notify/issues/3...

Styling will be like comments #10 or #12 in this same issue.

Fabulous, thanks! Confirmed this works at #3402293-15: Fix strict type errors: Convert FormattableMarkup to strings (simple replacement) in core/modules/*/tests/src/Kernel/* → , too.

Here's an example of someone coming on to an issue an pressing the hide button when they didn't mean to https://www.drupal.org/project/drupal/issues/3405798#comment-15343738 🐛 Config deleted during import does not have correct initial values set Needs review

I still think that this needs a confirm form as we've placed the hide / show buttons in exactly the same area of the issue as the show/hide files button and the show commands button. Both of these do make any permanent changes to the issue and only change things for the current user's current page.

Fixing attribution.

Thank you so much for this!

A confirm form would be a good followup since we’ve seen people misunderstand this. Could be either https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm or a Drupal confirm page.

Automatically closed - issue fixed for 2 weeks with no activity.