-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
🎮Implement the functionality to create and edit blog posts #39
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 10 11 +1
Lines 118 222 +104
Branches 6 5 -1
==========================================
+ Hits 118 222 +104 ☔ View full report in Codecov by Sentry. |
d561880
to
007f604
Compare
<div class="container"> | ||
<h1><a href="/">Django Girls Blog</a></h1> | ||
</div> | ||
<div class="container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change the indentation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What did you edit exactly and how?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry I didn't understand what I should correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore the original indentation, I suppose.
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
@webknjaz in this PR a lot of non-informative commits, I'll do a squash in the last push. |
django-girls-blog-OlenaEfymenko on 18-12-django-forms-new-edit via 🐍 3.11.2 using ☁️ default/fertu-pipeline-dev via django-girls-blog-OlenaEfymenko took 2.2s
➜ python -m coverage run manage.py test
Traceback (most recent call last):
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/manage.py", line 73, in <module>
main()
File "/home/olenayefymenko/.pyenv/versions/3.11.2/lib/python3.11/contextlib.py", line 81, in inner
return func(*args, **kwds)
^^^^^^^^^^^^^^^^^^^
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/manage.py", line 69, in main
execute_from_command_line(sys.argv)
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
utility.execute()
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 275, in fetch_command
klass = load_command_class(app_name, subcommand)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 48, in load_command_class
module = import_module("%s.management.commands.%s" % (app_name, name))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/olenayefymenko/.pyenv/versions/3.11.2/lib/python3.11/importlib/__init__.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen importlib._bootstrap>", line 1206, in _gcd_import
File "<frozen importlib._bootstrap>", line 1178, in _find_and_load
File "<frozen importlib._bootstrap>", line 1149, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 940, in exec_module
File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/core/management/commands/test.py", line 6, in <module>
from django.test.runner import get_max_test_processes
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/test/__init__.py", line 3, in <module>
from django.test.client import AsyncClient, AsyncRequestFactory, Client, RequestFactory
File "/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/django/test/client.py", line 727
session = await self.asession()
^^^^^^^^^^^^^^^^^^^^^
SyntaxError: 'await' outside async function
/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/coverage/inorout.py:519: CoverageWarning: Module blog was previously imported, but not measured (module-not-measured)
self.warn(msg, slug="module-not-measured")
/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/coverage/inorout.py:519: CoverageWarning: Module mysite was previously imported, but not measured (module-not-measured)
self.warn(msg, slug="module-not-measured")
/home/olenayefymenko/django-girls-blog-OlenaEfymenko/.venv/lib/python3.11/site-packages/coverage/control.py:887: CoverageWarning: No data was collected. (no-data-collected)
self._warn("No data was collected.", slug="no-data-collected") |
@webknjaz thank you for review, I'll change and push |
Edit tests
This patch provides property tests after the last review and includes additional logic for user login and logout. Tests
54f3d36
to
938264b
Compare
</header> | ||
<main class="container"> | ||
<main class="content container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this relate to editing the posts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps perform package version bumps in a separate PR?
kwargs={'pk': new_post.pk}, | ||
), | ||
) | ||
self.assertEqual(new_post.title, 'Test Title') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit difficult to connect the dots when this constant string is defined here, but the data it compares to is defined elsewhere. Why not reuse the same value saved into a var?
self.assertContains( | ||
http_response, 'This field is required.', html=True, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try using the assertFormError()
helper:
self.assertEqual(post.title, 'Updated Title') | ||
self.assertEqual(post.text, 'Updated Text') | ||
self.assertEqual(post.author, self.user) | ||
self.assertNotEqual(post.published_date, initial_published_date) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is a useful check, the best way of dealing with the time changes is freezing it. There are libraries like freezegun and similar that monkey-patch the stdlib that could be used. But since you did it manually in the past already, I suggest repeating the same technique here: https://github.com/kpi-web-guild/django-girls-blog-OlenaEfymenko/blob/f6ccd09/tests/test_views.py#L60-L63.
self.assertNotEqual(post.published_date, initial_published_date) | ||
|
||
def test_post_edit_invalid_form_authorized(self): | ||
"""Test handling of invalid form submission when editing a post.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to avoid using the vague "test doing something generic and abstract" and focus on declaring the expectation. The outcome of this test is probably not that you executed a test for the sake of executing a test that mentions handling the form. Instead, you probably expect that using a form in a specific way would result in a specific reaction of the server, don't you? Why not mention that specific thing in the docstring?
This PR:
Link to task https://tutorial.djangogirls.org/en/django_forms/
Ref #18