Skip to content

provide event class and typed event to LDAP loaded event; also fixing a logged deprecation message - #22221

Merged
blizzz merged 1 commit into
masterfrom
fix/19097/ldap-depracted-event
Aug 14, 2020
Merged

provide event class and typed event to LDAP loaded event; also fixing a logged deprecation message#22221
blizzz merged 1 commit into
masterfrom
fix/19097/ldap-depracted-event

Conversation

@blizzz

Copy link
Copy Markdown
Member

fixes#19097

I am not aware of any current consumers. The event is not very usable as it is fired before most apps are loaded. To keep things stable and avoid surprises nevertheless, this is the safe, backportable approach (code diverted a bit, but no biggy). With 20 and the IBootstrap bits from @ChristophWurst it should be a viable approach to subscribe to the typed event.

@blizzz

Copy link
Copy Markdown
MemberAuthor

/backport to stable19

@blizzz

Copy link
Copy Markdown
MemberAuthor

/backport to stable18

@blizzz
blizzzforce-pushed the fix/19097/ldap-depracted-event branch 2 times, most recently from 3658f08 to 698a884CompareAugust 12, 2020 13:38
* and also dispatch the typed event as current approach to it
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
blizzzforce-pushed the fix/19097/ldap-depracted-event branch from 698a884 to 44cad17CompareAugust 12, 2020 13:38
@faily-bot

faily-botBot commented Aug 12, 2020

Copy link
Copy Markdown

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 31703: failure

acceptance-app-files

  • tests/acceptance/features/app-files.feature:262
Show full log
 Scenario: unmarking a file as favorite causes the file list to be sorted again # /drone/src/tests/acceptance/features/app-files.feature:262
Given I am logged in # LoginPageContext::iAmLoggedIn()
And I create a new folder named "A name alphabetically lower than welcome.txt" # FileListContext::iCreateANewFolderNamed()
And I see that "A name alphabetically lower than welcome.txt" precedes "welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()
And I close the details view # FilesAppContext::iCloseTheDetailsView()
And I see that the details view is closed # FilesAppContext::iSeeThatTheDetailsViewIsClosed()
And I mark "welcome.txt" as favorite # FileListContext::iMarkAsFavorite()
And I see that "welcome.txt" is marked as favorite # FileListContext::iSeeThatIsMarkedAsFavorite()
And I see that "welcome.txt" precedes "A name alphabetically lower than welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()
When I unmark "welcome.txt" as favorite # FileListContext::iUnmarkAsFavorite()
Then I see that "welcome.txt" is not marked as favorite # FileListContext::iSeeThatIsNotMarkedAsFavorite()
Not favorited state icon for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)
And I see that "A name alphabetically lower than welcome.txt" precedes "welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()

@MorrisJobke

Copy link
Copy Markdown
Member

TBH: I couldn't find any users of this event. Let's just remove it? Why should we add an event where there is no use case? If there is one we can still add it properly.

Comment threadapps/user_ldap/lib/AppInfo/Application.php
@blizzz

Copy link
Copy Markdown
MemberAuthor

TBH: I couldn't find any users of this event. Let's just remove it? Why should we add an event where there is no use case? If there is one we can still add it properly.

There is, but the way it is pre-20 is not working, because the event is fired to early due to app loarding order.

@MorrisJobke

Copy link
Copy Markdown
Member

There is, but the way it is pre-20 is not working, because the event is fired to early due to app loarding order.

Where?

@blizzz

Copy link
Copy Markdown
MemberAuthor

There is, but the way it is pre-20 is not working, because the event is fired to early due to app loarding order.

Where?

ldap_write_support (and upstream) would have taken advantage of it

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a deprecation notice to #20953 for 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' and that the new event should be used.

@blizzz
blizzz merged commit 6e8e34f into masterAug 14, 2020
@blizzz
blizzz deleted the fix/19097/ldap-depracted-event branch August 14, 2020 14:07
@blizzz

Copy link
Copy Markdown
MemberAuthor

Please add a deprecation notice to #20953 for 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' and that the new event should be used.

done

@MorrisJobke

Copy link
Copy Markdown
Member

Please add a deprecation notice to #20953 for 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' and that the new event should be used.

done

I added it to the table we have in that ticket

@blizzz

Copy link
Copy Markdown
MemberAuthor

Please add a deprecation notice to #20953 for 'OCA\\User_LDAP\\User\\User::postLDAPBackendAdded' and that the new event should be used.

done

I added it to the table we have in that ticket

oh, that's better. (life's too short to scroll down though 😳 )

blizzz added a commit that referenced this pull request Aug 19, 2020
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
blizzz added a commit that referenced this pull request Aug 19, 2020
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
blizzz added a commit that referenced this pull request Aug 19, 2020
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@MorrisJobke

Copy link
Copy Markdown
Member

ref #14552

MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Oct 29, 2020
See nextcloud/server#22221
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request Oct 29, 2020
See nextcloud/server#22221
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsfeature: ldaptechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecated event type for OCA\User_LDAP\User\User::postLDAPBackendAdded: null

3 participants

@blizzz@MorrisJobke@rullzer