Skip to content

AA-33: Function to uniquely create calendar event id for user and assignment - #23141

Merged
Dillon-Dumesnil merged 1 commit into
masterfrom
ddumesnil/create-calendar-event-id-AA-33
Feb 21, 2020
Merged

AA-33: Function to uniquely create calendar event id for user and assignment#23141
Dillon-Dumesnil merged 1 commit into
masterfrom
ddumesnil/create-calendar-event-id-AA-33

Conversation

@Dillon-Dumesnil

Copy link
Copy Markdown
Contributor

No description provided.

@Dillon-Dumesnil Dillon-Dumesnil changed the title AA-33: Function to uniquely hash user and assignment AA-33: Function to uniquely create calendar event id for user and assignment Feb 19, 2020
@Dillon-Dumesnil
Dillon-Dumesnil force-pushed the ddumesnil/create-calendar-event-id-AA-33 branch from 6cf6834 to cd407bd Compare February 19, 2020 18:03
@Dillon-Dumesnil

Copy link
Copy Markdown
Contributor Author

jenkins run python

@mikix mikix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calendar is a very broad name. I get that it's directly tied to a calendar. But you could also see the schedule djangoapp name used for this or being confused with calendar. Is there a more specific name we could use?

ics_calendar is too tech specific (and probably an ATM machine issue).

calendar_sync maybe?

I dunno. Whatever, it's great.

Comment thread openedx/features/calendar/api.py Outdated
@@ -0,0 +1,19 @@
"""
API for calendar syncing Course dates with a User.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it our custom to hide these functions inside an api.py? That's fine, but wouldn't we want to import the names into init or something, so users don't have to import from an api file? Seems redundant from their POV.

@Dillon-Dumesnil
Dillon-Dumesnil force-pushed the ddumesnil/create-calendar-event-id-AA-33 branch 4 times, most recently from 21ca128 to 750eba3 Compare February 20, 2020 20:12

@mikix mikix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needn't block this PR, but blocks can have multiple "kinds" of dates, yeah? I think we mostly focus on the due type, but it's technically possible to have more?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. updated

@Dillon-Dumesnil
Dillon-Dumesnil force-pushed the ddumesnil/create-calendar-event-id-AA-33 branch from 750eba3 to 4892653 Compare February 20, 2020 20:38
The function will create a unique event id using a user and assignment
@Dillon-Dumesnil
Dillon-Dumesnil force-pushed the ddumesnil/create-calendar-event-id-AA-33 branch from 4892653 to 57dbb2d Compare February 21, 2020 16:44
@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@Dillon-Dumesnil
Dillon-Dumesnil merged commit e738d04 into master Feb 21, 2020
@Dillon-Dumesnil
Dillon-Dumesnil deleted the ddumesnil/create-calendar-event-id-AA-33 branch February 21, 2020 18:13
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/deploy_to_stage

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants