Avery Genetti - Front Page

wavepool/templates/wavepool/frontpage.html

@@ -29,7 +29,7 @@ <h2>Cover story</h2>
 			<div class="row">
 				<ul class="feed">
 					{% for newspost in top_stories %}
-						<li data-newspost-id="{{newspost.pk}}" data-top-story-placement="{{forloop.counter}}">
+						<li class="topstory" data-newspost-id="{{newspost.pk}}" data-top-story-placement="{{forloop.counter}}">
 							<span class="label label--soft">
 								{{newspost.publish_date}}
 							</span>
@@ -50,7 +50,7 @@ <h2>Cover story</h2>
 			<div class="row">
 				<ul class="feed">
 				{% for newspost in archive %}
-					<li data-newspost-id="{{newspost.pk}}" data-top-story-placement="{{forloop.counter}}">
+					<li class="archived-story" data-newspost-id="{{newspost.pk}}" data-top-story-placement="{{forloop.counter}}">
 							<span class="label label--soft">
 								{{newspost.publish_date}}
 							</span>

wavepool/tests.py

@@ -141,21 +141,26 @@ def test_top_stories(self):
         front_page = self.client.get('')
         front_page_html = BeautifulSoup(front_page.content, 'html.parser')
 
-        rendered_top_stories = front_page_html.find_all('div', {'class': 'topstory'})
+        # I changed this test definition because it felt like an error in the test.
+        # In the frontpage.html file, it appears that the element that this test is
+        # looking for is actually implemented as a <li> and not a <div>. Changing it
+        # to a div in the template file causes the test to pass, but it affects the rendering
+        # of the page with respect to the CSS styles, and doesn't seem like intended behavior.
+        rendered_top_stories = front_page_html.find_all('li', {'class': 'topstory'})
         self.assertEqual(len(rendered_top_stories), 3)
 
         top_story_1 = front_page_html.find(
-            'div', {'class': 'topstory', 'data-top-story-placement': '1', }
+            'li', {'class': 'topstory', 'data-top-story-placement': '1', }
         )
         top_story_1_id = int(top_story_1['data-newspost-id'])
 
         top_story_2 = front_page_html.find(
-            'div', {'class': 'topstory', 'data-top-story-placement': '2', }
+            'li', {'class': 'topstory', 'data-top-story-placement': '2', }
         )
         top_story_2_id = int(top_story_2['data-newspost-id'])
 
         top_story_3 = front_page_html.find(
-            'div', {'class': 'topstory', 'data-top-story-placement': '3', }
+            'li', {'class': 'topstory', 'data-top-story-placement': '3', }
         )
         top_story_3_id = int(top_story_3['data-newspost-id'])
 
@@ -179,10 +184,13 @@ def test_archive_stories(self):
 
         front_page = self.client.get('')
         front_page_html = BeautifulSoup(front_page.content, 'html.parser')
-        archive_story_divs = front_page_html.find_all('div', {'class': 'archived-story'})
-        self.assertEqual(len(archive_story_divs), len(archive_stories))
-        for div in archive_story_divs:
-            story_id = int(div['data-archive-story-id'])
+
+        # Similar to above, I modified the test to look for <li> elements and the 'data-newspost-id' property since
+        # this seems to match better with my best guess at the intended behavior, judging off of the template file.
+        archive_story_lis = front_page_html.find_all('li', {'class': 'archived-story'})
+        self.assertEqual(len(archive_story_lis), len(archive_stories))
+        for li in archive_story_lis:
+            story_id = int(li['data-newspost-id'])
             self.assertIn(story_id, [s.id for s in archive_stories])
 
     def test_newspost_teaser_render(self):

wavepool/views.py

@@ -14,9 +14,70 @@ def front_page(request):
             archive: the rest of the newsposts, sorted by most recent
     """
     template = loader.get_template('wavepool/frontpage.html')
-    cover_story = NewsPost.objects.all().order_by('?').first()
-    top_stories = NewsPost.objects.all().order_by('?')[:3]
-    other_stories = NewsPost.objects.all().order_by('?')
+
+    # "The newspost designated as the cover story should appear in the cover story box"
+    # ------
+    # Current implementation is to get the cover story by getting a random result.
+    #
+    # Need to change this to look for a post with is_cover_story = True.
+    #
+    # Also, it's unclear if the intended behavior is for there to only ever (and always)
+    # be one story with 'is_cover_story' set to True, or if we are to account
+    # for multiple/zero stories that meet this criteria. If I was designing the spec for 
+    # this app myself, I would consider allowing for any number of posts to be designated
+    # as cover stories; this way we would keep track of all historical cover stories and 
+    # not lose that information when a new story was chosen to be the covery story.
+    # Additionally, it seems intuitive to me that, were there multiple cover stories,
+    # the one to show on the front page would be the most recently published one.
+    #
+    # This has a drawback, however, which is that designating a new cover story that was
+    # published less recently than the current cover story would be impossible.
+    #
+    # If we wanted to solve that problem instead, there is an example of a way to implement 
+    # a unique True constraint in the NewsPost model in this Stack Overflow answer: 
+    # https://stackoverflow.com/a/61326996
+    #
+    # In that case, I could also simplify the below code a bit in the 'else' case to something like:
+    # cover_story = NewsPost.objects.get(is_cover_story=True) 
+    # if I could be assured that there was a maxiumum of one cover story.       
+
+    all_cover_stories = NewsPost.objects.all().filter(is_cover_story = True)
+
+    if all_cover_stories.count() == 0 :
+        # Handling the case where there is no story designated as the cover story by
+        # using the most recent story.
+        cover_story = NewsPost.objects.all().order_by('-publish_date').first()
+    else :
+        cover_story = NewsPost.objects.all().filter(is_cover_story = True).order_by('-publish_date').first() 
+
+    # "The 3 most recent stories, excluding the cover story, should be displayed 
+    # under "top stories", ordered by most recent first"
+    # ------
+    # Current implementation is to get 3 random stories.
+    #
+    # Need to change this to retrieving the 3 most recent stories that are not the cover story
+    #
+    # A note on how I implemented this - I'm specifically using the id from the cover_story instance variable
+    # because I'm considering the possibility that we're showing a de facto cover story that isn't actually marked as
+    # is_cover_story = true in the database; if we took the approach of insisting on is_cover_story = true being a unique
+    # constraint in the db, we could do filter(is_cover_story = False) for a similar result.
+
+    top_stories = NewsPost.objects.all().exclude(id = cover_story.id).order_by('-publish_date')[:3]
+
+    # "All news posts that do not appear as the cover story or as top stories should be listed in the archive, 
+    # ordered by most recent first"
+    # ------
+    # Current implementation is to list all stories randomly.
+    # 
+    # Need to order stories chronologically and exclude top stories + the cover story.
+    #
+    # This one is pretty straightforward; just build a list of ids to exclude from our existing variables (top_stories and cover_story)
+    # and make sure to order the posts correctly.
+
+    exclude_from_other_stories = [top_story.id for top_story in top_stories]
+    exclude_from_other_stories.append(cover_story.id)
+
+    other_stories = NewsPost.objects.all().exclude(id__in = exclude_from_other_stories).order_by('-publish_date')
 
     context = {
         'cover_story': cover_story,