Skip to content

Add a new method to remove an entry #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amazon-q-developer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a new method to remove an entry #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion flaskr/flaskr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def close_db(error):
@app.route('/')
def show_entries():
db = get_db()
cur = db.execute('SELECT title, text FROM entries ORDER BY id DESC')
cur = db.execute('SELECT id, title, text FROM entries ORDER BY id DESC')
Copy link
Author

@amazon-q-developer amazon-q-developer bot Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description: The SQL query in show_entries() fetches all entries without pagination, which may lead to performance issues with large datasets. Consider implementing pagination for the entries query to limit the number of results returned.

Severity: Medium

Copy link
Author

@amazon-q-developer amazon-q-developer bot Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix implements pagination for the entries query in the show_entries() function. It limits the number of results returned by using LIMIT and OFFSET in the SQL query, based on the current page number and entries per page. This addresses the performance issue with large datasets by fetching only a subset of entries at a time.

Suggested change
cur = db.execute('SELECT id, title, text FROM entries ORDER BY id DESC')
@app.route('/')
def show_entries():
db = get_db()
page = request.args.get('page', 1, type=int)
per_page = 10 # Number of entries per page
offset = (page - 1) * per_page
cur = db.execute('SELECT id, title, text FROM entries ORDER BY id DESC LIMIT ? OFFSET ?',
(per_page, offset))
entries = cur.fetchall()
return render_template('show_entries.html', entries=entries, page=page)
@app.route('/add', methods=['POST'])

entries = cur.fetchall()
return render_template('show_entries.html', entries=entries)

Expand All @@ -76,6 +76,17 @@ def add_entry():
return redirect(url_for('show_entries'))


@app.route('/delete/<int:id>', methods=['POST'])
def delete_entry(id):
if not session.get('logged_in'):
abort(401)
db = get_db()
db.execute('DELETE FROM entries WHERE id = ?', [id])
db.commit()
flash('Entry was successfully deleted')
return redirect(url_for('show_entries'))


@app.route('/login', methods=['GET', 'POST'])
def login():
error = None
Expand Down
16 changes: 16 additions & 0 deletions flaskr/static/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,22 @@ input[type="submit"]:hover {
background: var(--secondary-color);
}

/* Delete entry form styling */
.delete-entry {
display: inline-block;
margin-top: 0.5em;
}

.delete-entry input[type="submit"] {
background: var(--accent-color);
font-size: 0.9em;
padding: 0.5em 1em;
}

.delete-entry input[type="submit"]:hover {
background: #c0392b;
}

/* Responsive adjustments */
@media (max-width: 840px) {
.page {
Expand Down
9 changes: 8 additions & 1 deletion flaskr/templates/show_entries.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@
{% endif %}
<ul class=entries>
{% for entry in entries %}
<li><h2>{{ entry.title }}</h2>{{ entry.text|safe }}
<li>
<h2>{{ entry.title }}</h2>{{ entry.text|safe }}
{% if session.logged_in %}
<form action="{{ url_for('delete_entry', id=entry.id) }}" method=post class=delete-entry>
<input type=submit value="Delete" onclick="return confirm('Are you sure you want to delete this entry?');">
</form>
{% endif %}
</li>
{% else %}
<li><em>Unbelievable. No entries here so far</em>
{% endfor %}
Expand Down
41 changes: 41 additions & 0 deletions tests/test_flaskr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,47 @@ def test_show_entries(self):
# the database state is not guaranteed. In a real-world scenario,
# you might want to set up a known database state before running this test.

def test_delete_entry_unauthorized(self):
"""
Test that an unauthorized user cannot delete entries.
"""
with app.test_client() as client:
Copy link
Author

@amazon-q-developer amazon-q-developer bot Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description: The test functions could benefit from using pytest fixtures for setup and teardown. Consider using pytest fixtures to set up the test client, login state, and database entries.

Severity: Low

Copy link
Author

@amazon-q-developer amazon-q-developer bot Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix implements pytest fixtures for client, authentication, and database initialization. These fixtures improve test organization, reduce code duplication, and ensure proper setup and teardown for each test. The client fixture provides a test client, auth fixture handles authentication, and init_database fixture initializes and cleans up the database for each test. The test functions are updated to use these fixtures, improving their reliability and maintainability.

Suggested change
with app.test_client() as client:
import pytest
@pytest.fixture
def client():
return app.test_client()
@pytest.fixture
def auth(client):
return AuthActions(client)
@pytest.fixture
def init_database():
with app.app_context():
init_db()
yield
# Clean up the database after the test
db = get_db()
db.execute('DELETE FROM entries')
db.commit()
def test_show_entries(client):
"""
Test the show_entries function to ensure it correctly renders the template
with the entries from the database.
"""
# Make a GET request to the root URL
response = client.get('/')
# Check if the response status code is 200 (OK)
assert response.status_code == 200
# Check if the response contains the expected content
assert b'<title>Amazon Q Developer Flask Demo</title>' in response.data
# Remove the check for <h2>Entries</h2> as it's not in the template
# Note: We're not checking for specific entries here because
# the database state is not guaranteed. In a real-world scenario,
# you might want to set up a known database state before running this test.
def test_delete_entry_unauthorized(client):
"""
Test that an unauthorized user cannot delete entries.
"""
# Try to delete an entry without being logged in
response = client.post('/delete/1', follow_redirects=True)
# Should get a 401 Unauthorized response
assert response.status_code == 401
def test_delete_entry_authorized(client, auth, init_database):
"""
Test that an authorized user can delete entries.
"""
# First, log in
auth.login()
# Add an entry to delete
client.post('/add', data={
'title': 'Test Entry to Delete',

# Try to delete an entry without being logged in
response = client.post('/delete/1', follow_redirects=True)

# Should get a 401 Unauthorized response
assert response.status_code == 401

def test_delete_entry_authorized(self):
"""
Test that an authorized user can delete entries.
"""
with app.test_client() as client:
# First, log in
client.post('/login', data={
'username': app.config['USERNAME'],
'password': app.config['PASSWORD']
})

# Add an entry to delete
client.post('/add', data={
'title': 'Test Entry to Delete',
'text': 'This entry will be deleted'
})

# Get the page to find the entry ID
response = client.get('/')

# Now delete the entry (assuming it's the first one with ID=1)
response = client.post('/delete/1', follow_redirects=True)

# Check if deletion was successful
assert response.status_code == 200
assert b'Entry was successfully deleted' in response.data

# Verify the entry is no longer on the page
response = client.get('/')
assert b'Test Entry to Delete' not in response.data


class AuthActions(object):
Expand Down