Skip to content

Remove dependency on nette/security#132

Merged
dg merged 1 commit into
nette:masterfrom
enumag:security
May 3, 2016
Merged

Remove dependency on nette/security#132
dg merged 1 commit into
nette:masterfrom
enumag:security

Conversation

@enumag

@enumag enumag commented May 1, 2016

Copy link
Copy Markdown
Contributor

As far as I can tell nette/security seems to be optional everywhere in nette/application.

@dg

dg commented May 2, 2016

Copy link
Copy Markdown
Member

Can you change commit message to smthng like composer: removed …?

@enumag

enumag commented May 2, 2016

Copy link
Copy Markdown
Contributor Author

@dg Done.

@dg

dg commented May 2, 2016

Copy link
Copy Markdown
Member

Btw it is required by storeRequest() & restoreRequest()

@enumag

enumag commented May 2, 2016

Copy link
Copy Markdown
Contributor Author

@dg Added some conditions to make them work without user.

@dg

dg commented May 2, 2016

Copy link
Copy Markdown
Member

Did you test it?

@enumag

enumag commented May 2, 2016

Copy link
Copy Markdown
Contributor Author

Oh right... can't use getUser because it throws an exception... Added a test for storeRequest as well. There are no existing tests for restoreRequest.

Can't really test it in a real application though. I use my own implementation of these methods to make them work with object parameters.

@Majkl578

Majkl578 commented May 3, 2016

Copy link
Copy Markdown
Contributor

Needs rebase after force-pushes... :(

@dg dg merged commit bc2dfc2 into nette:master May 3, 2016
@dg

dg commented May 3, 2016

Copy link
Copy Markdown
Member

Thanks, merged

} while (isset($session[$key]));

$session[$key] = [$this->getUser()->getId(), $this->request];
$session[$key] = [$this->user ? $this->user->getId() : null, $this->request];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NULL

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, @dg already fixed that when he merged it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants