yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

Move user-related stuff to module advanced app #4734

Closed dynasource closed 9 years ago

dynasource commented 10 years ago

I have seen an enormous effort to enhance the advanced application to make it ready for public. Part of this focus is to create a product that reduces the amount of questions and to kickstart new users as fast a possible (please correct me if I'm wrong).

Another advantage of the advanced app is that people can adopt a project standard which they can use to develop projects that are consistent and logically built up. There are examples for configuration of backend/frontend, examples for tests etc. etc.

With respect to these 2 advantages, I think it would be logical to include a setup of a module. Most people out here use Modules to structure their Apps. @samdark has given some sort of direction over here https://github.com/yiisoft/yii2/issues/1520 but I think this topic deserves more attention.

Yii is developed to be the most flexible and modular. A convention on the structuring of Modules seem very logical to me. Especially in the Advanced application, the absence of a module example is like giving a child Lego without its manual.

To come back on my first sentences. Why not enhancing the advanced app with a module example that fits the advanced app structure.

Opinions about moving user-related stuff into module, added by @samdark

Do it

samdark commented 10 years ago

I think module example doesn't belong to application template. The demo should be a separate thing.

dynasource commented 10 years ago

I've seen a lot of people working on their own 'user' modules already (and so am I). They had to transfer the current implementation into a module.

If that is what's happening already, why not putting the User stuff into a module? Excellent example to the public to show how modules can be implemented. It is consistent with Yii's modular vision.

samdark commented 10 years ago

There no sense in extracting anything to a module if it will not be re-distributed. So either we're making user module an extension or leaving it as is.

dizews commented 10 years ago

it will be useful to describe (best practice) how to make models of modules more independent and isolated

samdark commented 10 years ago

Correct. That's why it belongs to demo app and not to the application template.

lynicidn commented 10 years ago

@dizews only use Yii::$app->getModule('staticId')->moduleClass in ur model

lynicidn commented 10 years ago

@dizews https://github.com/yiisoft/yii2/issues/4270#issuecomment-52173583

dizews commented 10 years ago

@lynicidn I think we should not mix levels of abstractions.I mean lower level should not call methods of higher level.

dynasource commented 10 years ago

There no sense in extracting anything to a module if it will not be re-distributed

Is that so? What about:

If we are not hardwiring this User functionality through modules into the basic/advanced templates , we neglect the fact that User functionality could be quite dynamic and isolated as @dizews mentiones.

We are talking about a template here. What would a template be without an example of one of its most commonly used aspects, the 'module' functionality. It fosters good programming and for the people that want to make use of the 'advanced template', module-implementation will be the first thing they will look for.

samdark commented 10 years ago

a module enables isolation of content & logic

Any example?

a module enables configuration & facilitates flexibility

Flexibility needed only when using a thing multiple times differently. In case of no-redistribution I don't see why flexibility is required withing a single application. There are cases when module may be used more than once in a single application but these are rare.

a module enables easy and fast (dis)connection of functionality

Same can be achieved by using if(Yii::$app->params['enableFeatureX']).

The template isn't meant to be example, it's actual template that is used to start a real project. In this real project it's very likely that module won't be used at all.

psihius commented 10 years ago

@samdark I kind'a agree with people on this one. Moving user related stuff to a user module is not as far fetched thing for an advanced template as it seems. The usage of an advanced template usually means this is not going to be a simple application and first thing you gonna do is heavily append to the user management part - 2 factor auth, social network auth, password restoration via mail/2 factor auth/whatever, user profile and so on. It becomes quite big very fast and moving it to a module is usually what it ends up anyway. So, on one side I understand that templates are usually kept as simple as possible, but in this case there is a yii2-simple template to do just that. I think on yii2-advanced we can go further and provide some basic module structure from the get go.

At least, in my opinion, it makes sense :) It just removes that tedious part of moving stuff yourself to a module. It's just like the RBAC part for Yii 1.1 - it is not in the template, but any sizable project with a backed ends up using it most of the time (all of my projects ended up with RBAC + srbac module ) :)

samdark commented 10 years ago

@yiisoft/core-developers more opinions?

dynasource commented 10 years ago

@Samdark, to anwer your question I refer to the arguments of @psihius. Sorry if I put you to extra work (like with the large codeception patch), its not my primary goal to keep you busy.

samdark commented 10 years ago

That's fine. I love to be busy, just want to make sure it's actually useful and reasonable.

tadaszelvys commented 10 years ago

I'd vote for more documentation on modules (or maybe new thing app-demo) instead of adding stuff to app-advanced or app-basic, for me even now app-advanced and app-basic has too much stuff. I'd leave only site/index (which would render a page with links to documentation) and get rid of everything else, its a template for YOUR app not a demo.

klimov-paul commented 10 years ago

Splitting application into a modules or use single solid solution is a yet another "holy var". There are different opinions on this matter, actual choice depends on the particular development workflow and organization.

Some people say application should always be split into "logical isolated" modules like "user" module mentioned here. These approach usually prefered by developers familiar with such systems as Joomla, Drupal, WordPress and so on. Such approach sounds good from high-level architecture point of view. But in real life creating really isolated(!) 'user' module is an utopia.

My own expirience of creating custom web application proves that real project requirements will mess any separated modules coupling them to each other. For example: imagine we have created 'user' module it allows login, forgot password and diplaying user profile data. Then we have created a separated module for "shopping cart". And after that, the customer demands information about user shopping cart should be displayed at user profile. Thus our isolated(!) user module have to go inside another isolated(!) shopping cart module to retrieve the information about user shopping cart.

Module usage make application more complex: you have several controller paths, several view paths, some pages may be rendered using parts of different modules (just read the example above). So where is a benefit?

Extracting module makes sense, if it really can be reused at other project. For example you may create a module, which manages application i18n messages. It can be isolated and reused because it does not actually belongs to the particular project logic. Such thiings like user login, forgot password and so on may seem to be universal, but they are not. What if I want to add a CAPTCHA to login form? What if want to log all login attempts? What if I want to block user who entered wrong password 3 times sequently?

Usage of module makes source code more complicated - I prefer keep application templates as simple as possible. They are the start point for the future development and should remain as light-weight sceletons. If you wish to create your own application template, which will widely use module - you are free to do it - create your own repository, composer package and so on.

dynasource commented 10 years ago

for me even now app-advanced and app-basic has too much stuff

Therefore modularizing stuff makes sense.

its a template for YOUR app not a demo

I agree. Therefore I was suggesting to do this for the User related stuff which is already in the advanced app. See https://github.com/yiisoft/yii2/issues/4734#issuecomment-52464146

Such approach sounds good from high-level architecture point of view

And it sounds good from many other views. See the arguments of https://github.com/yiisoft/yii2/issues/4734#issuecomment-52470716 and @psihius https://github.com/yiisoft/yii2/issues/4734#issuecomment-52471953

Module usage makes the application more complex

Modules can be used to standardize and to isolate. This fosters simplicity. Perhabs it is complex in the beginning (because you have to think more ahead before you code), but in the end it could save time & resources. Why not compare an application with an analogy in real life: IKEA.

Such thiings like user login, forgot password and so on may seem to be universal, but they are not.

Some of these can be made universal. If not, if there is a unique case, then you make a module called 'user-module-with-specific-functionality'. Perhabs with an overriding Module.

I prefer keep application templates as simple as possible

We are talking about the advanced template over here. Of course we should follow the KISS principle, but a little more depth in the advanced template is to be expected.

nineinchnick commented 10 years ago

Thinking about it more I'm against introducing it as a module, because as I said, the user module is not appropriate for introducing module concept alone. As @klimov-paul described, it requires a lot of funcionality and I personally don't believe we can all come to an agreement how to implement it correctly. The number of existing user modules validate that statement.

I'm pro teaching to use modules, but against using an user module to do that, because I don't think a proper official user module can be built.

Is there any other example for a module?

psihius commented 10 years ago

About the module re-usability, isolation and stuff:

@klimov-paul is right about the fact, that in reality you can't really isolate modules from each-other or make the app not to rely on some modules. I think we have here some misunderstandings from both sides on what can and cannot be achieved and what is a User module in this exact context. First, I want to make an emphasis on the fact, that we are talking about an application template. Template is meant to be a starting point, at witch you start to add/change the template. Second, modules are great for pulling together logical parts of the sizable application - at some point you just get too much controllers, too much models, templates and other stuff, that nicely fits into a module with it's own components, helpers and multiple controllers. This leads to Third: this is not a fully-fledged module we are talking about. We are talking about a basic skeleton for a User module that everyone will build the way they want and need it, or just replace it with a community module straight up from the start and be done with it.

I know from my personal experience - if it ain't there from the start, you end up re-factoring the damn application later anyway into modules. It's just how the laziness works :) I want to emphasize that we are talking about, and only about, the advanced template. This template is not for beginners who barely know the framework. Getting a grip on environment configuration the first time in this template requires an effort compared to understanding a basic module with a controller, model and a template.

klimov-paul commented 10 years ago

at some point you just get too much controllers, too much models, templates and other stuff

My opinion such problem may exist only for controllers. Models and components can be easy composed into sub folder. Do you really need to create full scale module to compose 'User', 'UserExtrernalLogin' and 'UserExtendedProfile' models into single package? Why you can not create a sub-folder at 'app/models', so you will have app\models\user\User and app\models\user\ExtrernalLogin instead of app\models\User and app\models\UserExtrernalLogin? I have seen an examples when developer in pursuit of 'pulling together logical parts of the sizable application' created a module, which consist of single controller with single action and single view.

My expirence shows me that separating modules for "pulling together logical parts of the sizable application" leads to tight coupling between these modules. In my previous example I have model user\models\User - from the 'user' module and shop\models\ShoppingCart from 'shop' module, while these models actually have a relation (foreigh key constrait) with each other: use has a shopping cart an shopping cart belongs to a user. Copling like this makes navigating the project to be very uneasy, especially searching for the particular view file. If I need to adjust the view, which renders count of items at user shopping cart, should I search for it at 'shop' module or at 'user' module? And when it comes to complex and large projects as you describe it, things even getting worse.

I know from my personal experience - if it ain't there from the start, you end up re-factoring the damn application later anyway into modules.

I have never done such a thing - never have a reason for it. I have composed models into sub-folders, extracted components and so on, but never created useless modules. Also think about this: why do you need to create an extra class for module if does not do anything useful?

I think we have here some misunderstandings from both sides on what can and cannot be achieved and what is a User module in this exact context.

Of course there could be a situation, when splitting application into modules makes sense. If your development team is familiar with this approach and have deep understanding and "feeling" of such appraoch - you may gain benefit from it. Am not telling anyone it is absolutely wrong or unacceptable. Here, I am trying to say that this is not universal approach even for complex applications. Some people beleave it is, while others disagree.

talking about, and only about, the advanced template

Advanced template serves mainly as an example of database usage and complex application configuration. This is not some "collection of all complex examples and approches" - it is covered by documentation. Splitting application into modules is a development workflow appraoch, which is possible and acceptable - but not necessary!

lynicidn commented 10 years ago

i think need add support for related models. http://symfony.com/doc/current/cookbook/doctrine/resolve_target_entity.html

klimov-paul commented 10 years ago

Relates #3146

dynasource commented 10 years ago

Thank you all for the time taken to form an opinion on this matter.

This discussion is moving towards a discussion 'where to draw the line' for using modules. Both ways have their (dis)advantages. I am a strong advocate of isolation. For me this belief outweighs any other arguments.

lynicidn commented 10 years ago

:+1:

freetuts commented 10 years ago

I would only move user related stuff from site controller to user controller, but there is no need to create module. Maybe it would be nice if advanced app ships with RBAC gui.

schmunk42 commented 10 years ago

I strongly agree with @dynasource and @psihius here.

Having the user (and also auth/RBAC) management related code in the application template has one big downside, which is that you can not really update this code automatically, especially if there are security issues. So one of the most crucial parts of your application has to be maintained manually(!).

On first sight it looks like that with the current approach it's easier to implement your custom changes for the login and profile logic, but this is also an easy task, if the user management would be a separate module extension. In fact it would be much easier, because you could start off with the standard module, and if you need a custom implementation you could just remove the yiisoft/yii2-user (which does not exist yet) module registered as user in your app, and replace it with your custom implementation - that's what modules are meant for, IMHO. If you have the same controllers and actions (API), you do not have to go through you whole app and look for all the links and routes which would have to be adjusted. You could also just fork the extension into a private repo if needed.

For a large number of small to mid-scale projects a user module as a core-extension would be enough, see yii-user for Yii 1.1, which was one of the most popular extensions. We used that in like 80% of our applications made for customers - although not perfect - it was the most practical solution.

For my Yii 2 application template I replaced the user related code from the advanced template with the user module from dektrium, but this kinda sucks, because when I do merges from the advanced app again, I have to resolve a lot of conflicts manually, eg. remove the login, logout action from the SiteController and adjust the links in the menu.

I can see no downside in having a separate extension for the user management in a module. To the contrary, it is a perfect example why to use a module.

For sure the implementation should be minimalistic. But as mentioned above, modules can be replaced much more easily, since modules are self-contained "mini-applications" - or to say it differently applications are also modules.

Ragazzo commented 10 years ago

well, afaik some of this modules exists in other fw, like in ZF2 however usually it is not always the same in every project, so you need to adjust it. Could be a good idea overall, but without too much unneeded complex things

tom-- commented 10 years ago

My (not very humble) opinion is that abstracting user management into a reusable module is generally inadvisable. User management is core business logic and doesn't lend itself to this. The best you can do is put some of the functions that user management commonly needs into reusable extension classes (mostly Component classes I would think), RBAC and Security being examples. But these would not constitute a module in the Yii sense, i.e. a self-contained reusable sub-application.

The best effort I have seen in this direction is @nineinchnick 's yii-usr and yii2-usr. Despite its quality and thoughtful design, it fails to convince me there is much to gain from trying to abstract user management into framework-like open-source APIs. I fact, I think there is actual risk of doing harm in attempting it.

I think the best Yii can do is show a simple example in the context of the advanced app and don't even suggest that user management can be modularized. So I'm in the Don't do it camp.

Ragazzo commented 10 years ago

agree with @tom-- in thing that use and its domain context have bigger value than reuse , however as was noted some example like in yii2-advanced would be good , so users will not each time do some general stuff again and again . Also similar to devise in rails , but i very light )

schmunk42 commented 10 years ago

I still fail to see why moving the user management into a module should be a bad decision in any way.

I think we all agree that this code should be included in a way, which can either be modified, extended or replaced easily - whether you want to reuse it or not.

Therefore it is just logical to separate this code into a module, which can be completely disabled by changing one config setting.

The worst-case is to add a new user management somehow and forget to completely disable the current template code which allows you to create an account and login right after activation, since there's no approval process in the current implementation - but that is not the point here.

But what's very important is, that you have to know to check all the following file locations, when replacing or extending the user management:

[edit]

and I almost forgot:

This is more than 50% of the code of the whole advanced app template.

tom-- commented 10 years ago

@schmunk42 The way you present it makes me wonder that this stuff is being delivered with the main Yii framework. I had no idea the app encompasses so much sensitive stuff.

If that stuff were turned into a module, I would argue that it should not be in a yiisoft repo.

schmunk42 commented 10 years ago

@tom-- Thanks. I totally agree with your last comment :)

To me it would be not that important under which github account the user module repo is located. It may also be in common/modules, although I'd prefer an extension.

But more the following two facts:

Firstly, if I remove

'modules' => ['user' => '\namespace\for\user\Module']

I can not (accidently) access any of the controllers or models above anymore.

Secondly, all of the code above should be found under either

tom-- commented 10 years ago

Those are good points, @schmunk42, but my concern is deeper. If such a module were to exist:

schmunk42 commented 10 years ago

What is its scope? To be just an example? or to be a general user management module that we expect people to reuse rather than adapt or just get some ideas from?

Same as at the moment, it's depending your use case - it can be everything ;) Let's assume the advanced-app requires an extension foo/user which includes the current stuff from the adavanced app and registers itself in the application bootstrap process as a module user.

Use case 1: I do not want a user management at all

"OK, maybe you should have chosen the basic app template :) but anyway...".

Use case 2: I want to keep it "as is"

Fine you are done. Note: if you'd fix the version constraint here to i.e. 1.0.0 - it would never be updated.

Use case 3: I want to modify it as part of my application

Use case 4: I want to use a reuseable extension

Note: You can can either fork the code from foo/user to develop your custom version, while still having the opporunity to merge fixes from the original repo - or just pick one from packagist.

Who would be responsible for it? Are the maintainers really able to commit to long term support of such security sensitive software?

It's depending on your use case, but essentially it's the same as now.

Using the module in the advanced app represents a strong endorsement of it by the Yii core team? Are they happy with doing that?

To me it's just a matter of implementation.

Is the Yii core team responsible for the current implementation? Answering this is not really possible, because you can not update the user management code. You only get "a maintained" version when creating a new project.

As mentioned above the user module does not have to implement more functionality like the current code in the template.

cebe commented 10 years ago

@schmunk42 I think this question has to be asked from the developers point of view. Meaning what will we want to work on and maintain. Not from the point of the user. Having the user related stuff in the application definitively means that we are not responsible for maintaining it because there is no working upgrade flow. The question here is whether such a situation is acceptable because most users might just take the advanced app and use it right away without caring for the details about the user management.

tom-- commented 10 years ago

What is the goal of yii2-app-advanced?

I honestly don't know but I think it's important for this discussion.

samdark commented 10 years ago

It is boilerplate.

qiangxue commented 10 years ago

I think we should stop spending time discussing this further. It won't result in anything.

As you know, it takes significant time to create a well designed reusable extension (widget, module, or whatever) because you have to consider customization, upgradability, and many other factors. I don't think any core developer has the time to develop and maintain such a module at this moment. Even if we create one, it will be very unstable. We may consider doing this if in future there are very good proven user-contributed extension

tom-- commented 10 years ago

If the app is supposed to be boilerplate then the user management stuff shouldn't exist in it because that is not boilerplate.

In order to demonstrate use of the boilerplate with this user management stuff as the example you should show how that would be done in practice. Presumably it would mean starting with composer to get the boilerplate in place and then demonstrate how the user would add these example classes in such a way that composer update will work.

But I'm quite convinced by now that pushing the user management stuff into a module is a terrible idea.

samdark commented 10 years ago

Yes, there's a plan to create demo app and write tutorial about it.

schmunk42 commented 9 years ago

@qiangxue @samdark @cebe I was viewing it all the time from a developers point of view.

One last question: Would it be OK for you to move the stuff into a module in the application? I'd be up for a PR for this.

This way it is still part of the application (and you do not have to take care about updates), but for those who want to replace it, it would be much easier and cleaner, see my comments above.

samdark commented 9 years ago

I'd prefer templates to be w/o it since it adds more complexity than needed in a boilerplate.

qiangxue commented 9 years ago

I don't think so. Modules are mainly useful when they are self-contained and redistributable. It is not trivial to reach this goal for a user module. In fact, if there is such a user module, it is almost certain when you would have to modify its code directly when you use it in your application. So instead of providing an incomplete user module in the app template, it's better we keep things straightforward.

schmunk42 commented 9 years ago

So instead of providing an incomplete user module in the app template, it's better we keep things straightforward.

The user module is not incomplete at the moment, nor if it would be in a module.

As show here the current implementation is not straightforward IMO, because there are too many files in different locations the developer has to take care about. See attached screenshot to get an impression of the proposed changes, it's merely about moving files...

bildschirmfoto 2014-09-25 um 15 45 41

samdark commented 9 years ago

@schmunk42 why not just moving all these files into namespace? Why module?

schmunk42 commented 9 years ago

@samdark You mean, move everything to common/user?

If yes, I see a few disadvantages:

samdark commented 9 years ago

The module will be used as a container for everything about user so:

  1. You can add rules for URL manager in the main config... or just do require(path/to/user/rules). No need for bootstrap in this case.
  2. That could probably be a problem but since it's just one controller I see no problem keeping it in the app dir.
  3. Yes but application template is just a tempate. You're not going to update it after deploying it for the first time and it's not so good practice to keep unused code in your app. Don't need built-in user management, delete it.
schmunk42 commented 9 years ago
  1. You can add rules for URL manager in the main config... or just do require(path/to/user/rules). No need for bootstrap in this case.

No need to, right. It was just an example, Gii does a similar thing - it is also a module.

  1. That could probably be a problem but since it's just one controller I see no problem keeping it in the app dir.

I'd like it more if all the user code which belongs together would be combined in a module.

  1. Yes but application template is just a tempate. You're not going to update it after deploying it for the first time and it's not so good practice to keep unused code in your app. Don't need built-in user management, delete it.

Exactly, this what I am talking about :) At the moment you have to remove several files in several directories, sometimes in a directory with files you must not delete - in addition you have to manually edit the actions in the site controller. If the code is moved to a module as suggested, just delete that folder.

samdark commented 9 years ago

@schmunk42 in case of Gii it actually helps end user not to specify rules himself. It's very convenient since extension is redistributable which is not the case in in-app module.

If one is reusing same template again and again deleting and modifying stuff in the same way it's better to create your own template.

schmunk42 commented 9 years ago

I added a PR with the proposed changes, namespaces still have to be adjusted and tested. May I finish this? I see no downsides at all with this solution.