search

lonnieezell / myth-auth

One-stop Auth package for CodeIgniter 4
MIT License
637 stars 208 forks source link

Re-work the filters & Set the route name for routing in config #444

Closed mjamilasfihani closed 2 years ago

mjamilasfihani commented 3 years ago

Fix issue #443

mjamilasfihani commented 3 years ago

hang in there

mjamilasfihani commented 3 years ago

I removed the helper('auth') and use loop to check the current url, in case if user has change their route name they just change the array. is it okay?

mjamilasfihani commented 3 years ago

if we use the CI version start from v4.0.5 the function url_is() is available, but if we end the support for < v4.0.5 I can just use url_is()

mjamilasfihani commented 3 years ago

now we can use login filter in global :)

MGatner commented 3 years ago

Looking better! A few changes. Also please be sure to run the style fixer.

mjamilasfihani commented 3 years ago

Wait for a moment, I'll fix it

mjamilasfihani commented 3 years ago

How about this PR @MGatner ? Any suggestion for the code position?

MGatner commented 3 years ago

It's too bulky for me to do on mobile, been waiting for a chance on desktop! From what I can see it looks right. I also want to check why tests are failing - I think it is unrelated to this PR.

mjamilasfihani commented 3 years ago

Okay, I hope it'll fix soon

mjamilasfihani commented 3 years ago

Hello @MGatner how the PHPStan's status? Still not working?

MGatner commented 3 years ago

The PHPStan issues aren't related to this PR - probably an update to the tool itself caused this. I'm good with this change! @lonnieezell last look?

mjamilasfihani commented 3 years ago

@MGatner I am happy to hear that :)

mjamilasfihani commented 3 years ago

Okay I made a proper review this time! A few suggestions to clean up and make this a legit refactor 😉

Thanks for the review, I will try to figure it :)

it's funny, I just learned about abstract class and you give that suggestion 😂

lonnieezell commented 3 years ago

I have no problems with this PR, I think. I agree with @MGatner latest comments so go ahead and address those, though.

mjamilasfihani commented 3 years ago

you can count on me

mjamilasfihani commented 3 years ago

fix #453

mjamilasfihani commented 3 years ago

Any idea how I fail all the test :( @MGatner

MGatner commented 3 years ago

@mjamilasfihani This repos tests might be borked. I would like to switch the CI/CD to the new CodeIgniter Dev Kit which hopefully will be ready today or tomorrow.

mjamilasfihani commented 3 years ago

That's good. Anytime you can do it @MGatner

MGatner commented 2 years ago

Hi @mjamilasfihani - a 5-month hiatus, but I am almost ready to merge the DevKit updates. Are you around to rebase this and clean up any changes for a final review once #517 is in?

mjamilasfihani commented 2 years ago

Thanks @MGatner for the good news, I'm ready for this 🔥