Skip to content

Comments

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

Merged
blizzz merged 1 commit intomasterfrom
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 intomasterfrom
fix/19097/ldap-depracted-event

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Aug 12, 2020

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 blizzz added this to the Nextcloud 20 milestone Aug 12, 2020
@blizzz
Copy link
Member Author

blizzz commented Aug 12, 2020

/backport to stable19

@blizzz
Copy link
Member Author

blizzz commented Aug 12, 2020

/backport to stable18

@blizzz blizzz mentioned this pull request Aug 12, 2020
Closed
@blizzz blizzz force-pushed the fix/19097/ldap-depracted-event branch 2 times, most recently from 3658f08 to 698a884 Compare August 12, 2020 13:38
@blizzz
provide event class to LDAP loaded event to fix deprecation message
44cad17 
* and also dispatch the typed event as current approach to it

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz force-pushed the fix/19097/ldap-depracted-event branch from 698a884 to 44cad17 Compare August 12, 2020 13:38
@faily-bot
Copy link

faily-bot bot commented Aug 12, 2020

🤖 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
Member

MorrisJobke commented Aug 12, 2020

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.

MorrisJobke
MorrisJobke reviewed Aug 12, 2020
View reviewed changes
@blizzz
Copy link
Member Author

blizzz commented Aug 13, 2020

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
Member

MorrisJobke commented Aug 13, 2020

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
Member Author

blizzz commented Aug 13, 2020

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

MorrisJobke
MorrisJobke approved these changes Aug 13, 2020
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

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.

@ChristophWurst ChristophWurst mentioned this pull request Aug 14, 2020
Closed
@blizzz blizzz merged commit 6e8e34f into master Aug 14, 2020
@blizzz blizzz deleted the fix/19097/ldap-depracted-event branch August 14, 2020 14:07
@blizzz
Copy link
Member Author

blizzz commented Aug 14, 2020

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
Member

MorrisJobke commented Aug 14, 2020

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

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Aug 14, 2020
Closed
@blizzz
Copy link
Member Author

blizzz commented Aug 14, 2020

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 😳 )

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Aug 14, 2020
Closed
blizzz added a commit that referenced this pull request Aug 19, 2020
@blizzz
simplified backport of #22221, fixing a deprecation log message
e28b8a5 
Signed-off-by: Arthur Schiwon <[email protected]>
blizzz added a commit that referenced this pull request Aug 19, 2020
@blizzz
simplified backport of #22221, fixing a deprecation log message
c8956e8 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz mentioned this pull request Aug 19, 2020
Merged
blizzz added a commit that referenced this pull request Aug 19, 2020
@blizzz
simplified backport of #22221, fixing a deprecation log message
5b358e7 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz mentioned this pull request Aug 19, 2020
Merged
@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 29, 2020

ref #14552

MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Oct 29, 2020
@MorrisJobke
Document typed LDAP event for user and group backend
f293c29 
See nextcloud/server#22221

Signed-off-by: Morris Jobke <[email protected]>
This was referenced Oct 29, 2020
Merged
Merged
backportbot-nextcloud bot pushed a commit to nextcloud/documentation that referenced this pull request Oct 29, 2020
@MorrisJobke @backportbot
Document typed LDAP event for user and group backend
30c53c1 
See nextcloud/server#22221

Signed-off-by: Morris Jobke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzer rullzer rullzer approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@viniciuscb viniciuscb Awaiting requested review from viniciuscb

Assignees

No one assigned

Labels

3. to review Waiting for reviews feature: ldap technical debt

Projects

None yet

Milestone

Nextcloud 20

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