Exiting the function

Hi

I have this code
def destroy
@property = Property.find(params[:id])
IsAuthorized?(@property.user_id)
@property.destroy

respond_to do |format|
  format.html { redirect_to(properties_url) }
  format.xml  { head :ok }
end

end

def IsAuthorized?(id)
if current_user.id!= id
flash[:notice] = 'Not authorized ’
redirect_to(properties_url)
end
end

If a not authorized user calls destroy it stills calls
@property.destroy.
How can I prevent the destory function from calling @property.destroy
if the user is not authorized?

On Sun, Apr 25, 2010 at 5:16 PM, Mohammed A. [email protected]
wrote:

Hi

I have this code
def destroy
@property = Property.find(params[:id])
IsAuthorized?(@property.user_id)
@property.destroy

Mohammed, you can write the above as follows:

@property.destroy if IsAuthorized?(@property.user_id)

Good luck,

-Conrad

On Sun, Apr 25, 2010 at 5:20 PM, Conrad T. [email protected]
wrote:

Mohammed, you can write the above as follows:
@property.destroy if IsAuthorized?(@property.user_id)

Well, you could, if IsAuthorized? – which idiomatically should
be is_authorized? – returned a boolean value :slight_smile:

def IsAuthorized?(id)
if current_user.id!= id
flash[:notice] = 'Not authorized ’
redirect_to(properties_url)
end
end

e.g.

def is_authorized?(id)
current_user.id == id
end


Hassan S. ------------------------ [email protected]
twitter: @hassan

Conrad

I did you suggested but I got this message
Render and/or redirect were called multiple times in this action.
Please note that you may only call render OR redirect, and at most
once per action. Also note that neither redirect nor render terminate
execution of the action, so if you want to exit an action after
redirecting, you need to do something like “redirect_to(…) and
return”.

On 26 April 2010 01:16, Mohammed A. [email protected] wrote:

 format.xml  { head :ok }

If a not authorized user calls destroy it stills calls
@property.destroy.

Adding to the other replies to this, the fundamental problem is that
redirect_to (somewhat non-intuitively) is not like a goto command. It
initiates the redirect and then returns, so that IsAuthorized returns
and the destroy is called whatever happened within IsAuthorized.

Colin

Mohammed, you can write the above as follows:
@property.destroy if IsAuthorized?(@property.user_id)

Well, you could, if IsAuthorized? – which idiomatically should
be is_authorized? – returned a boolean value :slight_smile:

Personally I’d prefer authorized? - I never use is_…?, the is_ is just
fluff (and doesn’t really make it any easier to read out
loud/comprehend.

Cheers,

Andy

On 26 April 2010 13:09, Mohammed A. [email protected] wrote:

Conrad

I did you suggested but I got this message
Render and/or redirect were called multiple times in this action.
Please note that you may only call render OR redirect, and at most
once per action. Also note that neither redirect nor render terminate
execution of the action, so if you want to exit an action after
redirecting, you need to do something like “redirect_to(…) and
return”.

Please put your replies inline rather than at the top of the message,
then statements such as “I did as you suggested” make sense as they
follow the suggestion made.

Read the other replies on this thread if you have not already done so.
The problem, as explained in the error message above is that
redirect_to is not a goto command but a function call. Execution
returns from there and drops through to the following code and so hits
the respond_to code and redirects again, which is illegal.

Colin

Andy J. wrote:

Mohammed, you can write the above as follows:
@property.destroy if IsAuthorized?(@property.user_id)

Well, you could, if IsAuthorized? – which idiomatically should
be is_authorized? – returned a boolean value :slight_smile:

Personally I’d prefer authorized? - I never use is_…?, the is_ is just
fluff (and doesn’t really make it any easier to read out
loud/comprehend.

Yes. Idiomatic Ruby style would be authorized? .

Cheers,

Andy

Best,

Marnen Laibow-Koser
http://www.marnen.org
[email protected]