Skip to content

Magic link add link back to login#375

Closed
jlopes90 wants to merge 14 commits intocodeigniter4:developfrom
jlopes90:magic-link-add-link-back-to-login
Closed

Magic link add link back to login#375
jlopes90 wants to merge 14 commits intocodeigniter4:developfrom
jlopes90:magic-link-add-link-back-to-login

Conversation

@jlopes90
Copy link
Contributor

No description provided.

@jlopes90
Copy link
Contributor Author

Because of "feat: add filter permission and group" the branch is develop. I forget a lot of time to create branch.

@kenjis
Copy link
Member

kenjis commented Aug 14, 2022

This branch has conflicts that must be resolved.
Please rebase:

$ git fetch upstream
$ git rebase upstream/develop

Resolve conflict, and git add the file, and

$ git rebase --continue

@kenjis kenjis added stale Pull requests with conflicts enhancement New feature or request labels Aug 14, 2022
@jlopes90
Copy link
Contributor Author

imagem

I did something wrong, and it doesn't show upstream.

@jlopes90
Copy link
Contributor Author

imagem

@kenjis
Copy link
Member

kenjis commented Aug 15, 2022

Run git remove -v.

@jlopes90
Copy link
Contributor Author

imagem

@kenjis
Copy link
Member

kenjis commented Aug 15, 2022

Sorry, git remote -v.

@jlopes90
Copy link
Contributor Author

imagem

@jlopes90
Copy link
Contributor Author

I think I already solved it, I used git remote add upstream https://github.com/codeigniter4/shield.git -f

@datamweb
Copy link
Collaborator

@jlopes90 , I want you to write a unittest for this PR.
Writing a test for this PR is very simple.
I want you to get familiar with the test and this PR is the best place to start.
can help you:
https://codeigniter4.github.io/CodeIgniter4/testing/response.html

public function testDisplayLoggedIn(): void
{
$authenticator = service('auth')->getAuthenticator();
assert($authenticator instanceof Session);
$authenticator->login($this->user);
$this->user->addGroup('admin', 'beta');
$this->user->addPermission('users.create', 'users.edit');
$output = $this->collector->display();
$this->assertStringContainsString('Current Use', $output);
$this->assertStringContainsString('<td>Username</td><td>John Smith</td>', $output);
$this->assertStringContainsString('<td>Groups</td><td>admin, beta</td>', $output);
$this->assertStringContainsString('<td>Permissions</td><td>users.create, users.edit</td>', $output);
}

You can write the test in tests/Controllers/MagicLinkTest.php.
eg:

 public function testHasLinkBackToLogin(): void
{
// test code
}

Please do your best, and let me know if you can't.

@datamweb datamweb removed the stale Pull requests with conflicts label Aug 15, 2022
@jlopes90
Copy link
Contributor Author

#375 (comment)

Unfortunately, I tried to figure it out and I still don't understand how things work.

@datamweb
Copy link
Collaborator

#375 (comment)

Unfortunately, I tried to figure it out and I still don't understand how things work.

It's okay, you'll learn it eventually. In fact, this test is not important for this PR. Look here, it will help you a lot.

@datamweb
Copy link
Collaborator

@jlopes90, change it:

 </form>
   <p class="text-center"><a href="<?= url_to('login') ?>"><?= lang('Auth.backToLogin') ?></a></p>

@datamweb datamweb requested a review from kenjis August 17, 2022 12:48
@kenjis kenjis added the tests needed Pull requests that need tests label Aug 17, 2022
@jlopes90
Copy link
Contributor Author

oops, wrong duplicate commits

@kenjis
Copy link
Member

kenjis commented Aug 19, 2022

I don't know why you committed the duplicate commits.

Please don't use git merge on the PR branch.
See the workflow: https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md

@jlopes90 jlopes90 closed this Aug 19, 2022
@jlopes90 jlopes90 deleted the magic-link-add-link-back-to-login branch August 19, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request tests needed Pull requests that need tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants