Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to IBootstrap #702

Merged
merged 6 commits into from Aug 9, 2021
Merged

Move to IBootstrap #702

merged 6 commits into from Aug 9, 2021

Conversation

PVince81
Copy link
Member

@PVince81 PVince81 commented Aug 6, 2021

This makes sure that the late setup is correctly running and removes the
hack where it was added to routes.php.

  • try and port the init code to use the passed-in context where possible

@PVince81 PVince81 added the 2. developing Work in progress label Aug 6, 2021
@PVince81 PVince81 self-assigned this Aug 6, 2021
@PVince81
Copy link
Member Author

PVince81 commented Aug 6, 2021

so now I've moved everything to the boot() method except for the two event classes that I could register earlier.

now, while testing:

  • BUG: app whitelist not working: I can see dashboard app visible and access it and also the comments tab => for some reason IUserSession->getUser() is null, so the restriction is not in place => resolved by swapping initialization order
  • TEST: notification: use "migrate_guest_user_data" config setting, see README => notification appears for migrated user
  • TEST: creating guest + dropdown values
  • TEST: guest list in settings page
  • TEST: share autoaccept event
  • TEST: guests-main is loaded thanks to the event
  • TEST: first login hook triggered
  • TEST: post share hook triggered
  • TEST: user/group backend registration: guest users are visible in users page
  • TEST: cannot share with local users

@PVince81
Copy link
Member Author

PVince81 commented Aug 6, 2021

I've tested on master and on that branch IUserSession->getUser() returns non-null, so the problem is specific to the move to IBootstrap. So I'll debug deeper to find out when the latter is actually getting populated and why it's not in boot().
Maybe need a callback of sorts ?

Edit: solved, I mixed up the initialization order: 23c292b

@PVince81 PVince81 marked this pull request as ready for review August 9, 2021 04:35
This makes sure that the late setup is correctly running and removes the
hack where it was added to routes.php.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
where possible...

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
As a first step to move to using the IBootstrap contexts, this reduces
the number of calls to $this->container to make it easier to move to the
proper context's container once ready.

Deleted unused private functions.
Inlined functions that were only used once.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Move initialization code to boot() and use the provided contexts.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
This way the user from the session can be properly resolved.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the bugfix/noid/move-to-bootstrap branch from 23c292b to 35be9c0 Compare August 9, 2021 05:16
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Looks great 🐘

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants