I have tested this item ✅ successfully on 724a7be

Would a test by review be sufficient, too? By review this PR looks right to me, but I don't have any Webauthn authenticator set up right now.

@richard67 it would be ok for me as this is well established code

@richard67 it would be ok for me as this is well established code

Well established and the change is clear and easy to understand.

P.S.: ... and I had tested similar PRs successfully in past.

I have tested this item ✅ successfully on 724a7be

By review.

RTC

Thanks

This has actually broken the Edit button in the WebAuthn management interface. Had any of you made even a cursory test you would have known.

The problem is in line 114 of the manage.php file where you changed the TD to a TH.

The build/media_source/plg_system_webauthn/js/management.es6.js file is targeting the TD elements inside the TR, see lines 192 to 196. By changing the element to a TH you broke the Edit button.

@brianteeman You have been complaining for a very long time that a. people contribute untested code and b. core committers push the merge button without thinking. In this case you did not test your change, you asked core committers to hit the merge button without having any tests at all and you did not even bother to ping me about this PR even though I am the designated code owner for WebAuthn as per .github/CODEOWNERS.

This time y'all got lucky because I just so happened to stumble across it before it made it to production while I am refactoring the WebAuthn plugin for Joomla 4.2.

The whole reason of the PR SOPs and code ownership is to avoid this trifecta of failures and to ensure that catching these issues happens long before merging and not because the code owner oh-just-so-happened to be refactoring the code after the breakage was merged into the repo!

Please take the following corrective actions:

• Fix the management.es6.js file.
• Change the .github/CODEOWNERS to include me as the code owner of the layouts/plugins/system/webauthn/manage.php file since it was moved outside the plugin's folder.
• Never contribute untested code especially if you think you are too good / your change is too small to need any testing at all.
• Never set code to RTC without any tests whatsoever on the basis that the contributor pinky swears it's all good.
• Never merge code that has an owner without asking the code owner for feedback.

I just checked my branch and I had actually fixed the js - but stupidly only in /media and not build/media-source so it was not picked up by git. Mea culpa

I assume that it doesnt need correcting as you have fixed it in #37673

regarding the codeowners file - its not going to work how you expect it to work. gh designed it so that the person named in the file woulod automatically receive notification BUT that only works if that user is also a member of the repo.

No, it still needs fixing I'm afraid :( My PR is for Joomla 4.2 and not merged yet, your PR is merged and is for Joomla 4.1. If it's not fixed the next release of Joomla (4.1.3) will have a broken WebAuthn. If you could just copy over the same solution I have in my PR — which I made in a separate commit so you can do just that, by the way — it'd be very much appreciated.

Regarding the CODEOWNERS file, ugh, well, at least having it complete would be a good start. If the owners of the Joomla organisation on GitHub could add a read-only team and invite us code owners into it it'd solve the automation problem. Otherwise there's no point in having a code owner who's never notified preemptively but might still be called to fix what's broken after the fact. The whole point of code ownership is for us to share our domain-specific knowledge very early to avoid SNAFUs, am I right?

which I made in a separate commit so you can do just that, by the way

That's this one here: 84b996a

@brianteeman Will you make a PR? That would be great.

yes i will make a pr later today when i finish testng the new pr for 4.2

yes you are correct about the purpose of codeowners - its a limitation of github that I brought up in their internals but got no traction. Your proposed solution might work @HLeithner is it possible you take a look

I'm looking into the read only group and coordinate this with the other maintainers

Thank you @HLeithner

PR for my error #37676