-
Notifications
You must be signed in to change notification settings - Fork 0
Add sketch of client/server best practices document #7
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
Open
tildedave
wants to merge
1
commit into
master
Choose a base branch
from
client-server-best-practices
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# Business Logic on the Server, Presentation Logic on the Clients | ||
|
||
Avoid the clients needing to know business logic like "is the user an organizer of this campaign?" in order to display a button. Links are a good pattern to allow this. See [https://github.com/tilteng/api/wiki/Links-%26-Feature-Discovery]Links & Feature Discovery] for more information. | ||
|
||
At our best we are building APIs that allow clients to be as dumb and agnostic to the business as possible. More logic on the server allows us to consolidate logic between all three of our clients, minimizes the surface area for bugs, and allows us to change client behavior without forcing uses to download the newest version of the app. | ||
|
||
Example: | ||
|
||
Users can invite friends to a campaign using `POST /campaigns/:id/invites`. Users are going to invite friends through a button in the website and the mobile clients. | ||
|
||
However, campaigns that are expired can't have invites. We wouldn't want to show this button in the case that someone is viewing an expired campaign, so we'll want to hide it or otherwise gray it out. | ||
|
||
One way to accomplish this would be for the server to return `is_expired` as a flag in the campaign object, and have the client optionally display the button based on whether or not `is_expired` is set. | ||
|
||
However, this is a fragile design. If there are other reasons why the client needs to forbid invites (for example, a campaign has had inviting explicitly disabled), every client must now update. Additionally, since the server controls access to `POST /campaigns/:id/invites`, this logic must also be duplicated on the server. | ||
|
||
A better solution is whether the server presents the capability to invite to the client. One pattern to do this is by including the invite href as a "link" subobject on the campaign. Here are some example links: | ||
|
||
``` | ||
"campaign": { | ||
"links": { | ||
/* determine whether invite button should be shown */ | ||
"invites.create": { | ||
"href": "/campaigns/CMPB546BAC556AF42DEB45AC83C31169F0B/invites", | ||
"method": "POST" | ||
}, | ||
/* determine whether "remind all" button should be shown */ | ||
"reminders.create": { | ||
"method": "POST", | ||
"href": "/campaigns/CMPB546BAC556AF42DEB45AC83C31169F0B/invite_reminders" | ||
} | ||
} | ||
} | ||
``` | ||
|
||
The "remind all" feature is organizer-only. With this design, the client only needs to look for the presence of the "reminders.create" on the campaign object to know whether or not to add a button - which means that the client does not have to add logic about whether the logged in user is the organizer. This design easily supports other access levels with a minimum of client changes - if a client is coded only to display the button only on the presence of the "reminders.create" link, a "co-organizer" who is authorized to remind can be easily supported. | ||
|
||
# Single User Interaction -> Single Client Request | ||
|
||
Single button press - single API call | ||
Single view - constant number of API calls (avoid n+1 calls for table rows) | ||
|
||
# Performance | ||
|
||
Run DBIC_TRACE=1 DBIC_TRACE_PROFILE=console to understand what your routes are doing | ||
Run explain on your queries | ||
Avoid n+1 SQL calls | ||
Optimize the columns you fetch (join vs prefetch) | ||
|
||
# Error Messages | ||
|
||
Human-Readable | ||
|
||
# Pagination | ||
|
||
Use marker-based pagination whenever possible. This avoids the clients needing to do logic to construct what request to make for the "next page" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to think calling this "marker-based" is a misnomer. A better name might be "href-based" or "link-based". The term "marker-based" refers to a specific type of paging that uses the unique key of records to page through the set rather than numeric offsets. |
||
|
||
Sorting - ideally controlled by server-side | ||
Invites: for organizer, remindable people at top, joined at bottom |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
That link markup is broken
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.
oh bah