Return groups displayname in provisioning api

[skjnldsv]

Following #8255

Needed by #8824

[codecov]

Codecov Report

Merging #8865 into master will increase coverage by 0.01%.
The diff coverage is 90.9%.

@@ Coverage Diff @@## master #8865 +/- ##
============================================
+ Coverage 51.88% 51.89% +0.01% - Complexity 25283 25287 +4 
============================================
Files 1604 1604 Lines 94918 94929 +11 Branches 1388 1388 ============================================
+ Hits 49245 49262 +17 + Misses 45673 45667 -6

Impacted Files	Coverage Δ	Complexity Δ
apps/provisioning_api/appinfo/routes.php	0% <0%> (ø)	0 <0> (ø)	⬇️
...ovisioning_api/lib/Controller/GroupsController.php	92.85% <100%> (+1.19%)	25 <4> (+4)	⬆️
apps/files_trashbin/lib/Trashbin.php	72.7% <0%> (+0.24%)	136% <0%> (ø)	⬇️
core/js/js.js	65.85% <0%> (+0.55%)	0% <0%> (ø)	⬇️
lib/private/Files/Cache/Propagator.php	96.2% <0%> (+1.26%)	16% <0%> (ø)	⬇️

[MorrisJobke]

This breaks a public API - could you link it in the ticket that collects all of these changes? cc @rullzer@nickvergessen Is it fine to do the change?

[skjnldsv]

@MorrisJobke which ticket?
You mean the vue settings pr?

Also, wz definitely needs to support such data since last nickvergessen's pr. We should have implemented it at the time I think! :)

[MorrisJobke]

Also, wz definitely needs to support such data since last nickvergessen's pr. We should have implemented it at the time I think! :)

Sure, but it still breaks existing code, because ['groupID1', '$groupID2'] as return values is not the same as {'groupID1' => 'group name 1', 'groupID2' => 'group name 2'} 😉

You mean the vue settings pr?

no.

@MorrisJobke which ticket?

The ticket about breaking changes: #7827

[skjnldsv]

Indeed! So how should we do that? Do you want me to create another entry point? :)

[rullzer]

Mmm I would vote for a 'displayName' parameter or so. Defaulting to false. So we don't break the API. But when true returning this.

[nickvergessen]

Parameter sounds good to me

[nickvergessen]

Please dont break the api, but add a parameter which when set also returns the display name.

[skjnldsv]

@nickvergessen

Please dont break the api, but add a parameter which when set also returns the display name.

@rullzer finally agreed we should change this.

<rullzer> The problem I have with it is that for apps it is easy. They know on what version they run. But for "REST" api's it is really bad to change behaviour. Like we would need to bump the version
<rullzer> which is fine by me
<rullzer> just it is also what we do with the OCP api. We break it. But apps know it breaks on V14
<rullzer> here it is still v1
<rullzer> but it still breaks
<rullzer> that is just bad design
<rullzer> You see what I mean?
<skjnldsv> Okay, I understand! :)
<skjnldsv> Yes, we could indeed bump the versionning
<rullzer> I would eb fine by adding a v2 endpoint for this and slowly moving stuff over to cleaner functions
<skjnldsv> Isn't it already a v2?
<rullzer> no it is v2 of the OCP protocol
<rullzer> v1 of the API
<skjnldsv> Oh
<skjnldsv> confusing :p
<rullzer> aaah fuck the provisioning API doesn't have a versioning aspect
<MorrisJobke> xD
<rullzer> stupid stuff :S
<skjnldsv> ^^
<rullzer> k just break it. We add it to the release notes and be done with it

[MorrisJobke]

CI said no:

provisioning-v1.feature:184

😉

[skjnldsv]

Okay, I added a new endpoint like the users list.
/groups/details

It fit our url design better and avoid the breaking of our current api, sorry for the change @nickvergessen@rullzer! :)

[rullzer]

Would be nice to also cover all those new endpoints with integration tests btw ;)

[skjnldsv]

@rullzer I'll do it in a future pr while replacing the proper Exceptions with the non-deprecated ones :)

Also, I restarted, but failure is unrelated, test before rebase was successful on those who timed out on the curren tone :)