Comments (9)
Maybe split the controller action into 2?
And make the routes like:
/en/profile/id/123
/en/profile/name/tom
Thing to be aware of though is that usernames can change, if the user bundle allows it of course its just a case of editing ones account and changing username, which is why i prefer to address the profile by id.
from ccdnuserprofilebundle.
Good point, I'd definitely favour profile lookup by id then.
The only problem is: this behaviour has been exposed. Any applications using the bundle might have exposed URLs such as /en/profile/tom
. If so, this will create a backwards-compatibility break, as the URLs will no longer work.
from ccdnuserprofilebundle.
URLs should be generated via routes, devs should not be hard-coding urls. Only thing affected really is google caching etc.
As the routes are already named as *_by_id then its just a case of changing the route from:
ccdn_user_profile_show_by_id:
pattern: /{userId}
defaults: { _controller: CCDNUserProfileBundle:Profile:showOverview, userId: 0, _locale: en }
to
ccdn_user_profile_show_by_id:
pattern: /id/{userId}
defaults: { _controller: CCDNUserProfileBundle:Profile:showOverview, userId: 0, _locale: en }
ccdn_user_profile_show_by_name:
pattern: /name/{userName}
defaults: { _controller: CCDNUserProfileBundle:Profile:showOverviewByName, _locale: en }
Though, tbh, we can forgo the option to list by username altogether if its easier, though its a nice way to find a profile manually in the quickest way though it does require an additional method, or we can use the same method, with 2 variables $id, and $name and prioritise name over id when set to determine model method for profile retrieval.
from ccdnuserprofilebundle.
Assuming experienced Symfony 2 developers, then yes, URLs will never be hardcoded. But with the Router, you can generate a faulty URL:
// Inside a container-aware class.
// Outputs: /app_dev.php/en/profile/tom
echo $this->container
->get('router')
->generate('ccdn_user_profile_show_by_id', ['userId' => 'tom']);
So the possibility exists that someone uses the URL in their own application. If the possibility exists, isn't it a BC break?
In any case, maybe we should keep it simple for the moment:
- Find by Id, but not by name. It's useful, but too likely to change. And search functionality is much more common for that use case.
- Same with keeping the URL as-is, but prioritising one over the other. As long as usernames can be the same as a user's identifier, then there is room for confusion.
from ccdnuserprofilebundle.
I doubt it honestly. The variable states ID, so i doubt anyone is using username in there as i don't think its mentioned in the docs that you can.
I think its better to have a BC break at this early point to get it right rather than wait for more people to use it and compound the issue.
from ccdnuserprofilebundle.
That's a good assertion, I think. All right, so let's:
- replace all calls of the generic find method to use the find-by-id method, and,
- remove the generic find method.
from ccdnuserprofilebundle.
Sounds good.
from ccdnuserprofilebundle.
I'll close this ticket, as it's fixed in the merged PR (next time I'll need to find a way of attaching PRs to existing tickets)
from ccdnuserprofilebundle.
next time I'll need to find a way of attaching PRs to existing tickets
yeah you can reference as many ticket-numbers as you like from a PR either in the commit message or here in the discussion by prefixing them with a hash symbol, github will see them and link it all up for you.
from ccdnuserprofilebundle.
Related Issues (4)
- Docs are out-of-date HOT 3
- KnpPaginatorBundle dependency HOT 3
- Unwanted creation of profiles HOT 13
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ccdnuserprofilebundle.