- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 8
Resolves #32 - A guest or an authenticated user can list answers of a question #53
Resolves #32 - A guest or an authenticated user can list answers of a question #53
Conversation
…tps://github.com/ana-lisboa/api into FEAT/laravel-portugal#32/GetAnswersFromAQuestion
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.
Thanks for this @ana-lisboa! I've made a few notes to improve your work that I'd like you to tackle, before aligning this PR to be merged. 😉
        
          
                domains/Discussions/Controllers/QuestionsGetAnswersController.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                domains/Discussions/Controllers/QuestionsGetAnswersController.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                domains/Discussions/Controllers/QuestionsGetAnswersController.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                domains/Discussions/Controllers/QuestionsGetAnswersController.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
      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.
Now, this is looking really good. We just need to tackle the n+1 problem and the naming of the answers index route name, and we'll get this merged.
        
          
                domains/Discussions/Tests/Feature/AnswersIndexControllerTest.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                domains/Discussions/Tests/Feature/AnswersIndexControllerTest.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
      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.
We're getting closer to the final version, I believe. We need to see what's happening with the tests that are failing, and deal with some notes I left.
        
          
                domains/Discussions/Tests/Feature/AnswersIndexControllerTest.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                domains/Discussions/Tests/Feature/AnswersIndexControllerTest.php
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | Some more alterations done, from the feedback received. The first test is still failing. | 
| Thanks for this @ana-lisboa. Can you just update the CHANGELOG file withe the updates we're making, here? | 
| Absolutely!
Cumprimentos,
Ana Lisboa
A quinta, 19/11/2020, 09:46, José Postiga <[email protected]>
escreveu:…  Thanks for this @ana-lisboa <https://github.com/ana-lisboa>. Can you just
 update the CHANGELOG file withe the updates we're making, here?
 —
 You are receiving this because you were mentioned.
 Reply to this email directly, view it on GitHub
 <#53 (comment)>,
 or unsubscribe
 <https://github.com/notifications/unsubscribe-auth/AC4HBOXVK4SJV6KTTVH7323SQTSRHANCNFSM4TGZP6AQ>
 .
 | 
| 
 added! | 
…estion # Conflicts: # CHANGELOG.md
No description provided.