pycon.tw icon indicating copy to clipboard operation
pycon.tw copied to clipboard

fix(proposal): 404 error for co-speaker

Open SivanYeh opened this issue 10 months ago • 6 comments

Types of changes

Thanks for sending a pull request! Please fill in the following content to let us know better about this change. Please put an x in the box that applies

  • [x] Bugfix
  • [ ] New feature
  • [ ] Refactoring
  • [ ] Breaking change (any change that would cause existing functionality to not work as expected)
  • [ ] Documentation Update
  • [ ] Other (please describe)

Description

Resolve 404 error and strange style of table after co-speaker pressed accept/decline during reviewing stage, which "SLUG.proposals.editable" is false.

Steps to Test This Pull Request

Steps to reproduce the behavior:

  1. Create 2 users: main speaker A and co speaker B.
  2. Go to Django admin and set to cfp stage as doc
  3. Sign in as speaker A into http://localhost:8000/en-us/dashboard/ and create a new proposal
  4. Go to Django admin and set to review stage.
  5. Found the proposal from speaker A and add B in "Additional Speakers" table.
  6. Sign in as speaker B into http://localhost:8000/en-us/dashboard/ and press "accept" and "decline" on the request.
  7. See error if page 404 shows up.

Expected behavior

404 happened in Step[7] disappear.

More Information

Screenshots image

SivanYeh avatar Apr 14 '24 14:04 SivanYeh

@mattwang44 uncertain if it's a good solution: When I simply remove if/else statement in the dispatch function, 404 no longer shows up(of course) while speaker A remains unaffected(Did I miss anything?)

image image

SivanYeh avatar Apr 14 '24 14:04 SivanYeh

記得修改 test cases

mattwang44 avatar Apr 15 '24 10:04 mattwang44

記得修改 test cases

賀! 研究中

SivanYeh avatar Apr 15 '24 10:04 SivanYeh

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.99%. Comparing base (fca6fd2) to head (a018685). Report is 46 commits behind head on master.

:exclamation: Current head a018685 differs from pull request most recent head 6a6c2d5

Please upload reports for the commit 6a6c2d5 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1188      +/-   ##
==========================================
+ Coverage   71.19%   73.99%   +2.79%     
==========================================
  Files          84       81       -3     
  Lines        3451     3057     -394     
==========================================
- Hits         2457     2262     -195     
+ Misses        994      795     -199     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 24 '24 09:04 codecov[bot]

@mattwang44 I thought “request” can be one of parameters to switch the edit status and check if additional speaker is allowed to update/cancel the form during uneditable time.

...seems like “request” is not included in these two forms. (Based on the par “request” came from “RequestUserValidationMixin”, which is not indcluded in “AdditionalSpeakerUpdateForm”.)

What is the key par should I consider?

SivanYeh avatar May 20 '24 09:05 SivanYeh

@mattwang44 跟GPT協作找到了一個方法是用RequestFactory來測試: https://docs.djangoproject.com/en/5.0/topics/testing/advanced/ 見commit54bf911

SivanYeh avatar Jul 29 '24 12:07 SivanYeh