This content has been marked as final. Show 5 replies
there is no "active" column in your "login" query's recordset... you have "priv" column instead...
so try changing login.active to login.priv
oh, and another thing:
do not have "username" and "password" as arguments. same names are used for attributes in the <cffunction> tag, and that can cause problems. rename them to something like "uname" and "pwd" instead...
Thanks you for the heads-up on not selecting the active column in the table.
I added that is it worked fine.
I don't have any problems with using username and password as arguments.
Thanks for the heads up though, probably better to change it anyways.
glad i could help.
re "username" and "password" as function arguments:
you will run into problems if you try to pass any of those inline inside the <cfinvoke> tag when invoking the cfc that contains your function. that is, if it is in a cfc...
Sabaidee as covered why your code didn't work. here's some more pointers
where your code could be improved...
> <cffunction name="UserLogin" access="remote" returntype="void">
Set output="false" on all methods unless you actually are outputting stuff
from them (which generally you should not).
I doubt this method would be appropriate for calling as a webservice (given
it ends with a <cflocation>), so don't expose it as one. access="public".
Add hints to all your <cffunction> and <cfargument> tags. It assists
sanity checking and code documentation.
> <cfquery name="login" datasource="***">
You have not VARed the login variable. This is poor practice unless you
explicitly mean to not VAR it.
> WHERE username='#arguments.username#' AND password='#arguments.password#'
Always use <cfqueryparam> tags. It improves performance and reduces memory
consumption on your DB server.
> <cflock scope="session" timeout="30" type="exclusive">
> <cfset Session.MM_Username = #arguments.username#>
> <cfset Session.SitePriv = #login.priv#>
There's no need to lock this sort of variable assignment.
> <cflocation url="/clinic/contribute/main/index.cfm" addtoken="no">
It's pretty poor practice to use <cflocation> within a function. You CFM
page should call a method, and get a response back from it. And then the
CFM page should decide whether to <cflocation> or not. Possibly your
function should set the URL for the <cflocation> and return the value.
Then the calling code should use than in a <cflocation> call.
> <cfelseif login.active EQ 0>
This if/elseif/else construct would be better done as a switch. Rule of
thumb: evaluating the same variable for many conditions: switch.
evaluating different variables for many conditions: if/elseif/else.