Add edit_entry and delete_entry to flaskr #987

Closed
wants to merge 1 commit into
from

Conversation

3 participants
@hiroakis

No description provided.

@danieldiekmeier

This comment has been minimized.

Show comment Hide comment
@danieldiekmeier

danieldiekmeier Feb 27, 2014

Hi!

I'm not sure this addition would really make sense, since flaskr is only supposed to be a really small example for a flask app. I've never needed it (after I got the hang of flask).

Also, the way you handle the parameters is not optimal.

You're using

<a href="{{ url_for('delete_entry') }}?id={{ entry.id }}">delete</a>

but you should rather use

<a href="{{ url_for('delete_entry', id=entry.id) }}">delete</a>

This should work without changing any other code, but if you want it really clean, you could also edit your functions to start with

@app.route('/delete/<int:id>')
def delete_entry(id):
    // other code here

and use the variable id in your method, instead of request.args.get('id').

Hi!

I'm not sure this addition would really make sense, since flaskr is only supposed to be a really small example for a flask app. I've never needed it (after I got the hang of flask).

Also, the way you handle the parameters is not optimal.

You're using

<a href="{{ url_for('delete_entry') }}?id={{ entry.id }}">delete</a>

but you should rather use

<a href="{{ url_for('delete_entry', id=entry.id) }}">delete</a>

This should work without changing any other code, but if you want it really clean, you could also edit your functions to start with

@app.route('/delete/<int:id>')
def delete_entry(id):
    // other code here

and use the variable id in your method, instead of request.args.get('id').

@untitaker

This comment has been minimized.

Show comment Hide comment
@untitaker

untitaker Feb 27, 2014

Owner

I agree that this is out of scope, also i don't think GET requests are appropriate for this.

Owner

untitaker commented Feb 27, 2014

I agree that this is out of scope, also i don't think GET requests are appropriate for this.

@hiroakis

This comment has been minimized.

Show comment Hide comment
@hiroakis

hiroakis Feb 28, 2014

Hi there!

Thank you for your advice.
I close my pull request.

Hi there!

Thank you for your advice.
I close my pull request.

@hiroakis hiroakis closed this Feb 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment