fix: sessions cannot be edited after cfs ends #6757

codedsun · 2020-01-18T15:17:44Z

Fixes #6726

Short description of what this resolves:

fix: sessions cannot be edited after cfs ends

Checklist

I have read the Contribution & Best practices Guide and my PR follows them.
My branch is up-to-date with the Upstream development branch.
The unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

All the functions created/modified in this PR contain relevant docstrings.

mrsaicharan1

Did you test this locally?

codedsun · 2020-01-18T15:44:00Z

@mrsaicharan1 - Yes the CI is failing, I am fixing the dredd test

app/api/sessions.py

codecov · 2020-01-18T16:18:48Z

Codecov Report

Merging #6757 into development will decrease coverage by 0.01%.
The diff coverage is 43.75%.

@@               Coverage Diff               @@
##           development    #6757      +/-   ##
===============================================
- Coverage         65.4%   65.38%   -0.02%     
===============================================
  Files              300      301       +1     
  Lines            15337    15356      +19     
===============================================
+ Hits             10031    10041      +10     
- Misses            5306     5315       +9

Impacted Files	Coverage Δ
app/api/speakers.py	`47.42% <33.33%> (-0.45%)`	⬇️
app/api/sessions.py	`41.12% <33.33%> (-0.2%)`	⬇️
app/api/helpers/speaker.py	`50% <50%> (ø)`
app/api/schema/events.py	`91.53% <0%> (+0.06%)`	⬆️
app/models/event.py	`78.52% <0%> (+0.15%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1787ca...73ae8f9. Read the comment docs.

iamareebjamal · 2020-01-18T16:22:36Z

app/api/sessions.py

@@ -148,6 +150,11 @@ def before_update_object(self, session, data, view_kwargs):
        if session.is_locked and data.get('is_locked') == session.is_locked:
            raise ForbiddenException({'source': '/data/attributes/is-locked'}, "Locked sessions cannot be edited")

+        speakers_call = safe_query(self, SpeakersCall, 'event_id', session.event_id, 'event-id')
+        if speakers_call.ends_at.replace(tzinfo=None) <= datetime.now().replace(tzinfo=None):


Now I won't be able to delete the session as well.

Second, you have made it timezone unaware

@iamareebjamal - Shall the sessions be deleted after the CFS is ended?

Admin/Co-organizer should be able to edit and delete both

app/api/sessions.py

codedsun · 2020-01-18T19:34:24Z

@iamareebjamal - Review

kushthedude

Read the complete issue, Speaker details needs to be locked as well. Or if you are partially fixing than don't mention the fixes keyword!

codedsun · 2020-01-19T06:07:45Z

Read the complete issue, Speaker details needs to be locked as well

I have read the issue, the speaker details can't be locked currently as it's related to the user not the CFS of the event. Once the speaker is created the same profile is applied for all the cfs of every event.
@kushthedude

Edit: The above is not true as the speaker profile is generated everytime for every new event

kushthedude · 2020-01-19T06:21:51Z

Remove the keyword fixes then.

…

On Sun, 19 Jan, 2020, 11:37 Suneet Srivastava, ***@***.***> wrote: Read the complete issue, Speaker details needs to be locked as well I have read the issue, the speaker details can't be locked currently as it's related to the user not the CFS of the event. Once the speaker is created the same profile is applied for all the cfs of every event. @kushthedude <https://github.com/kushthedude> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6757?email_source=notifications&email_token=AKQMTLVGFT43OFLUTM6QYQ3Q6PU3FA5CNFSM4KITREOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJKJ4WY#issuecomment-575970907>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKQMTLRFGS7H3W4W3FU7R5LQ6PU3FANCNFSM4KITREOA> .

codedsun · 2020-01-19T06:45:12Z

@kushthedude - What I looked upon is for every event a new speaker profile is generated.

app/api/speakers.py

codedsun · 2020-01-19T09:07:30Z

@iamareebjamal @kushthedude @mrsaicharan1 - Please review. Thanks

codedsun · 2020-01-19T18:00:06Z

@iamareebjamal - Review please thanks.

iamareebjamal · 2020-01-19T18:58:39Z

app/api/speakers.py

+        if speakers_call and not (has_access('is_admin') or has_access('is_organizer', event_id=speaker.event_id) or
+                                  has_access('is_coorganizer', event_id=speaker.event_id)):
+            speakers_call_tz = speakers_call.ends_at.tzinfo
+            if speakers_call.ends_at <= datetime.now().replace(tzinfo=speakers_call_tz):


Duplicate logic

houndci-bot · 2020-01-20T14:14:20Z

app/api/speakers.py

 from app.models.session_speaker_link import SessionsSpeakersLink
 from app.models.user import User

+from datetime import datetime


'datetime.datetime' imported but unused

houndci-bot · 2020-01-20T14:14:20Z

app/api/speakers.py

 from app.api.schema.speakers import SpeakerSchema
 from app.models import db
 from app.models.event import Event
 from app.models.session import Session
 from app.models.speaker import Speaker
+from app.models.speakers_call import SpeakersCall


'app.models.speakers_call.SpeakersCall' imported but unused

houndci-bot · 2020-01-20T14:14:20Z

app/api/sessions.py

@@ -17,6 +18,8 @@
 from app.models.session_type import SessionType
 from app.models.speaker import Speaker
 from app.models.track import Track
+from app.models.speakers_call import SpeakersCall
+from datetime import datetime


'datetime.datetime' imported but unused

houndci-bot · 2020-01-20T14:14:21Z

app/api/sessions.py

@@ -17,6 +18,8 @@
 from app.models.session_type import SessionType
 from app.models.speaker import Speaker
 from app.models.track import Track
+from app.models.speakers_call import SpeakersCall


'app.models.speakers_call.SpeakersCall' imported but unused

houndci-bot · 2020-01-20T14:14:21Z

app/api/helpers/speaker.py

+        else:
+            return True
+    else:
+        raise ForbiddenException({'source' : '/data/event-id'}, 'Speaker Calls for event {id} not found'.format(id=event_id))


line too long (125 > 120 characters)
whitespace before ':'

houndci-bot · 2020-01-20T14:14:22Z

app/api/helpers/speaker.py

+        speakers_call_tz = speakers_call.ends_at.tzinfo
+        if speakers_call.ends_at <= datetime.now()\
+            .replace(tzinfo=speakers_call_tz) and not (has_access('is_admin')
+            or has_access('is_organizer', event_id=event_id) or has_access('is_coorganizer', event_id=event_id)):


continuation line under-indented for visual indent
visually indented line with same indent as next logical line

app/api/helpers/speaker.py

houndci-bot · 2020-01-20T14:14:22Z

app/api/helpers/speaker.py

@@ -0,0 +1,22 @@
+


Black would make changes.

houndci-bot · 2020-01-20T14:18:03Z

app/api/helpers/speaker.py

+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id).one()
+    if speakers_call:
+        speakers_call_tz = speakers_call.ends_at.tzinfo
+        if speakers_call.ends_at <= datetime.now().replace(tzinfo=speakers_call_tz)\


Black would make changes.

houndci-bot · 2020-01-20T14:22:48Z

app/api/helpers/speaker.py

+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id).one()
+    if speakers_call:
+        speakers_call_tz = speakers_call.ends_at.tzinfo
+        if speakers_call.ends_at <= datetime.now().replace(tzinfo=speakers_call_tz) \


Black would make changes.

codedsun · 2020-01-20T14:31:54Z

@iamareebjamal - Review now

iamareebjamal · 2020-01-20T16:26:23Z

app/api/helpers/speaker.py

+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id).one()
+    if speakers_call:
+        speakers_call_tz = speakers_call.ends_at.tzinfo
+        if speakers_call.ends_at <= datetime.now().replace(tzinfo=speakers_call_tz) \


Just use datetime.utcnow()

@iamareebjamal - Won't work

TypeError: can't compare offset-naive and offset-aware datetimes

We have to compare the current time (with the timezone of the speaker call) and the speaker call end date which already is timezone aware

iamareebjamal · 2020-01-20T16:27:47Z

app/api/helpers/speaker.py

+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id).one()
+    if speakers_call:
+        speakers_call_tz = speakers_call.ends_at.tzinfo
+        if speakers_call.ends_at <= datetime.now().replace(tzinfo=speakers_call_tz) \


Just return not of this expression, no need for explicit returns of boolean

codedsun · 2020-01-21T01:35:19Z

@iamareebjamal - Ping!

iamareebjamal · 2020-01-21T06:08:04Z

app/api/helpers/speaker.py

+    Method to check that user has permission to edit the speaker or session
+    after the CFS ends
+    """
+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id).one()


Will not filter out deleted speaker calls

houndci-bot · 2020-01-21T06:18:26Z

app/api/helpers/speaker.py

+    Method to check that user has permission to edit the speaker or session
+    after the CFS ends
+    """
+    speakers_call = SpeakersCall.query.filter_by(event_id=event_id, deleted_at=None).one()


Black would make changes.

niranjan94 · 2020-01-21T06:21:59Z

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- app/api/helpers/speaker.py  5

See the complete overview on Codacy

codedsun · 2020-01-21T06:23:09Z

@iamareebjamal - Review

codedsun · 2020-01-21T06:30:45Z

@iamareebjamal - Thankyou lord 🥇😆

auto-label bot added the fix label Jan 18, 2020

mrsaicharan1 reviewed Jan 18, 2020

View reviewed changes

mrsaicharan1 suggested changes Jan 18, 2020

View reviewed changes

app/api/sessions.py Outdated Show resolved Hide resolved

app/api/sessions.py Outdated Show resolved Hide resolved

codedsun force-pushed the sessions-edit branch from a77cb48 to c081efd Compare January 18, 2020 16:18

iamareebjamal reviewed Jan 18, 2020

View reviewed changes

codedsun force-pushed the sessions-edit branch from c081efd to 6673ac4 Compare January 18, 2020 19:08

houndci-bot reviewed Jan 18, 2020

View reviewed changes

app/api/sessions.py Outdated Show resolved Hide resolved

codedsun force-pushed the sessions-edit branch from 6673ac4 to 8a4c251 Compare January 18, 2020 19:11

codedsun force-pushed the sessions-edit branch from 8a4c251 to dbab61c Compare January 18, 2020 19:44

fix: sessions cannot be edited after cfs ends

ceb867e

codedsun force-pushed the sessions-edit branch from dbab61c to ceb867e Compare January 18, 2020 19:49

kushthedude suggested changes Jan 19, 2020

View reviewed changes

houndci-bot reviewed Jan 19, 2020

View reviewed changes

app/api/speakers.py Outdated Show resolved Hide resolved

app/api/speakers.py Outdated Show resolved Hide resolved

codedsun force-pushed the sessions-edit branch from e8965e9 to 865d84d Compare January 19, 2020 08:55

houndci-bot reviewed Jan 19, 2020

View reviewed changes

app/api/speakers.py Outdated Show resolved Hide resolved

codedsun force-pushed the sessions-edit branch 2 times, most recently from 70ed3e9 to 5f0755e Compare January 19, 2020 09:02

iamareebjamal reviewed Jan 19, 2020

View reviewed changes

codedsun force-pushed the sessions-edit branch from 5f0755e to 63d26c3 Compare January 20, 2020 14:14

houndci-bot reviewed Jan 20, 2020

View reviewed changes

codedsun force-pushed the sessions-edit branch from 63d26c3 to 9eba3ca Compare January 20, 2020 14:17

houndci-bot reviewed Jan 20, 2020

View reviewed changes

codedsun force-pushed the sessions-edit branch 2 times, most recently from 110d5d1 to 7236b59 Compare January 20, 2020 14:22

houndci-bot reviewed Jan 20, 2020

View reviewed changes

iamareebjamal reviewed Jan 20, 2020

View reviewed changes

codedsun force-pushed the sessions-edit branch from 7236b59 to 5921239 Compare January 20, 2020 17:33

iamareebjamal reviewed Jan 21, 2020

View reviewed changes

fix: speakers can't be edited after the cfs ends

73ae8f9

codedsun force-pushed the sessions-edit branch from 5921239 to 73ae8f9 Compare January 21, 2020 06:18

houndci-bot reviewed Jan 21, 2020

View reviewed changes

iamareebjamal approved these changes Jan 21, 2020

View reviewed changes

iamareebjamal merged commit d2d7c37 into fossasia:development Jan 21, 2020

iamareebjamal mentioned this pull request Feb 17, 2021

fix: Allow session and speaker update after CfS end #7750

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: sessions cannot be edited after cfs ends #6757

fix: sessions cannot be edited after cfs ends #6757

codedsun commented Jan 18, 2020

mrsaicharan1 left a comment

codedsun commented Jan 18, 2020 •

edited

Loading

codecov bot commented Jan 18, 2020 •

edited

Loading

iamareebjamal Jan 18, 2020

codedsun Jan 18, 2020

iamareebjamal Jan 18, 2020

codedsun commented Jan 18, 2020

kushthedude left a comment

codedsun commented Jan 19, 2020 •

edited

Loading

kushthedude commented Jan 19, 2020 via email

codedsun commented Jan 19, 2020 •

edited

Loading

codedsun commented Jan 19, 2020

codedsun commented Jan 19, 2020

iamareebjamal Jan 19, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

houndci-bot Jan 20, 2020

codedsun commented Jan 20, 2020

iamareebjamal Jan 20, 2020

codedsun Jan 20, 2020

codedsun Jan 20, 2020

iamareebjamal Jan 20, 2020

codedsun commented Jan 21, 2020

iamareebjamal Jan 21, 2020

houndci-bot Jan 21, 2020

niranjan94 commented Jan 21, 2020

codedsun commented Jan 21, 2020

codedsun commented Jan 21, 2020

fix: sessions cannot be edited after cfs ends #6757

fix: sessions cannot be edited after cfs ends #6757

Conversation

codedsun commented Jan 18, 2020

Short description of what this resolves:

Checklist

mrsaicharan1 left a comment

Choose a reason for hiding this comment

codedsun commented Jan 18, 2020 • edited Loading

codecov bot commented Jan 18, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codedsun commented Jan 18, 2020

kushthedude left a comment

Choose a reason for hiding this comment

codedsun commented Jan 19, 2020 • edited Loading

kushthedude commented Jan 19, 2020 via email

codedsun commented Jan 19, 2020 • edited Loading

codedsun commented Jan 19, 2020

codedsun commented Jan 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codedsun commented Jan 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codedsun commented Jan 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niranjan94 commented Jan 21, 2020

codedsun commented Jan 21, 2020

codedsun commented Jan 21, 2020

codedsun commented Jan 18, 2020 •

edited

Loading

codecov bot commented Jan 18, 2020 •

edited

Loading

codedsun commented Jan 19, 2020 •

edited

Loading

codedsun commented Jan 19, 2020 •

edited

Loading