Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V4 #327

Merged
merged 17 commits into from
Sep 9, 2020
Merged

V4 #327

merged 17 commits into from
Sep 9, 2020

Conversation

danhenderson
Copy link
Contributor

No description provided.

Copy link
Contributor

@rowanhogan rowanhogan left a comment

Choose a reason for hiding this comment

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

Just a couple points

page: activity.PageGuid,
slug: activity.PageShortName,
teamId: activity.TeamGuid,
message: activity.Title || activity.title,
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a tiny pet peeve, but I find these big methods are so much easier to read when the keys are sorted alphabetically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm usually the same. I think I had "title" on my mind there.

@danhenderson
Copy link
Contributor Author

As mentioned in Slack, there were a few things tripping me up, so I'm trying to consolidate things a little bit. Here are the main changes since you last saw it.

Title and description

We are in a weird state where title and description were mixed up between Strava and Manual, and they were also called different things (e.g. message, even though it was the title, not the message...very confusing).

I went through and made everything consistently title and description (I did allow caption to passed as a fallback for title just for a bit of backwards compatibility). I also updated the create fitness form to be able to includeDescription and includeTitle.

Activity Type vs Type

This was another that was mixed, this time between the Consumer API response and GraphQL response. So I moved them explicitly up to the top to set the source and activityType correctly.

ID vs External ID vs Legacy ID

Again, this was getting very confusing between Strava/Manual (via GraphQL or Consumer, depending on whether it is being returned by GraphQL or Consumer API. I have standardised this so regardless of if you are fetching from GraphQL or Consumer API (legacy), you get the same IDs in the same fields each time.

id: this is the timeline id (null in the case of legacy only manual fitness)
externalId: this is the Strava ID
legacyId: this is the Consumer API id (only available if useLegacy: true and can be used to delete old activities)

NOTE: most of the above is specific to the JG implementation, and the only change to EDH is to follow suit w/ title and description. The id is still always the fitness entry id for EDH (ahh, simpler times).

Copy link
Contributor

@rowanhogan rowanhogan left a comment

Choose a reason for hiding this comment

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

👍

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.

2 participants