feat(docs/api): Add proposed endpoints for swags by GeekyShacklebolt · Pull Request #30 · CuriousLearner/nexus

GeekyShacklebolt · 2019-08-22T19:04:50Z

Fix endpoints of "proposals" app to support "lightning talks".
Add another endpoint in "proposals" app to notify speakers if their
proposal is accepted.
Add proposed endpoints for "swags" app.

* Fix endpoints of "proposals" app to support "lightning talks". * Add another endpoint in "proposals" app to notify speakers if their proposal is accepted. * Add proposed endpoints for "swags" app.

CuriousLearner

Please find my comments inline.

CuriousLearner · 2019-08-22T19:45:09Z

 title              | text          | Title of proposal
-speaker            | text          | Speaker for the talk
-kind               | text          | Type of proposal like talk, dev sprint, workshop
+speaker            | UUID          | Foreign Key to `User` Model


The client doesn't care if you're using SQL and have a foreign key. It is just user_id for the user :)

CuriousLearner · 2019-08-22T19:46:26Z


+__Note__:
+- For `lightning_talk`, duration will be fixed like `00:05:00`, etc. which would be configuration through settings.
+- For `lightning_talk`, `level` of proposal is like speaker's experience which can be `beginner`, `intermediate` or `advanced`.


A person can have a different experience in different domains. You cannot assume it as constant. Also, anyone from an audience can be a speaker for lightning talks.

CuriousLearner · 2019-08-22T19:47:25Z

+## Send notification to speaker
+
+```
+POST /api/proposal/:id/notify (requires authentication)


I might not want to send individual notifications.

CuriousLearner · 2019-08-22T19:48:19Z

+**Request**
+```json
+{
+    "name": "Tshirt",


this should be all lower case. There should be a mapping for FE on what to send here.

Internally this would be a choice field. The mapping can be provided in a meta-data API.

Providing internal choice for such a broad key like name might not be possible because name can be anything like tshirt, cap, thumb drive, mug, notebook, chargers, etc. What we can do is to create a key category with possible general choices like wearables, devices, kit, bag and other (while leaving the key name as a common text_field). This way it would be more flexible. What do you think?

CuriousLearner · 2019-08-22T19:48:39Z

+## Update swag details
+
+```
+PATCH /api/swag/:swag_id (requires authentication)


Suggested change

PATCH /api/swag/:swag_id (requires authentication)

PATCH /api/swag/:id (requires authentication)

CuriousLearner · 2019-08-22T19:50:12Z

+}
+```
+
+# User-Swag


What is User-Swag?

UserSwag is a through-table to relate Users with Swags as a many-to-many relation. All the following endpoints are dealing on this model. It will exist in the Swags app itself and its usage lies in marking if a Swag is given to a User and the time when it is given.

Why are we having an entry in the API for swag when the swag is not given yet?

Right! We don't need to create an entry if the swag is not given yet. Following this, we won't be needing is_given, because the creation of the entry will itself mean that the swag is given and created_at will be the same as given_at. So we can remove both is_given and given_at.

CuriousLearner · 2019-08-22T19:51:21Z

+## Upload image of a swag
+
+```
+POST /api/swag/:swag_id/upload_image (requires authentication)


All these APIs should have a plural form like */api/swags/*

* Add key "item" in model "Swag" as a foreign key to hold "SwagItems". These items will behave like choices for creating the swag.

gutsytechster · 2019-08-30T13:03:10Z

 status             | text          | Status of proposal like `retracted`, `accepted`, `unaccepted`, `submitted`, etc.

+__Note__:
+- For `lightning_talk`, duration will be fixed like `00:05:00`, etc. which would be configuration through settings.


Suggested change

- For `lightning_talk`, duration will be fixed like `00:05:00`, etc. which would be configuration through settings.

- For `lightning_talk`, the duration will be fixed like `00:05:00`, etc. which would be configurable through settings.

gutsytechster · 2019-08-30T13:06:44Z

 {
-    "title": "Corrected title of talk",
-    "level": "advanced"
+    "id": "0f342ac1-ac32-4bd1-3612-efa32bc3d9a0",


Since it's a PATCH request and we are providing id in the URL itself, no need to keep it in the request then.

gutsytechster · 2019-08-30T13:09:44Z

+**Request**
+```json
+{
+    "id": "0f342ac1-ac32-4bd1-3612-efa32bc3d9a0",


Same as above for id here. :)

gutsytechster · 2019-08-30T13:17:49Z

+```
+
+**Response**
+Status: 201 Created


Shouldn't a PATCH return a 200 Ok status as it doesn't create any resource rather modify an existing resource?

Yes, it should be 200 OK.

gutsytechster · 2019-08-30T13:19:19Z

+**Request**
+```json
+{
+    "id": "0f342ac1-ac32-4bd1-3612-efa32bc3d9a0",


Same as above for id :)

CuriousLearner

I have a few suggestions and questions.

Please find my review inline.

CuriousLearner · 2019-09-07T21:07:15Z

+## Send notification to individual speaker
+
+```
+POST /api/proposals/:id/notify (requires authentication)


This can be either acceptance/rejection.

CuriousLearner · 2019-09-07T21:08:41Z

+```json
+{
+    "id": "0f342ac1-ac32-4bd1-3612-efa32bc3d9a0",
+    "item": "12abcff0-bcbc-01fc-abcd-01012bc4e0b1",


why an id here? Please show the resource here.

CuriousLearner · 2019-09-07T21:09:07Z

+    "id": "0f342ac1-ac32-4bd1-3612-efa32bc3d9a0",
+    "item": "12abcff0-bcbc-01fc-abcd-01012bc4e0b1",
+    "description": "sponsered by someone",
+    "image": null,


Make it as URL so it is pretty evident for someone integrating the API on what to expect.

CuriousLearner · 2019-09-07T21:10:05Z

+```json
+{
+    "item": "12abcff0-bcbc-01fc-abcd-01012bc4e0b1",
+    "description": "sponsered by someone else",


Just provide a few fields here.
Also, what are an item and a swag exactly?

CuriousLearner · 2019-09-07T21:11:09Z

+## Delete the image of swag
+
+```
+DELETE /api/swags/:id/delete_image (requires authentication)


This can be POST.

CuriousLearner · 2019-09-07T21:12:01Z

+**Response**
+Status: 200 OK
+```json
+{


If this is just one item, why not give a name to swag altogether rather than trying to link them through a foreign key?

feat(docs/api): Add proposed endpoints for swags

0f50423

* Fix endpoints of "proposals" app to support "lightning talks". * Add another endpoint in "proposals" app to notify speakers if their proposal is accepted. * Add proposed endpoints for "swags" app.

CuriousLearner requested changes Aug 22, 2019

View reviewed changes

GeekyShacklebolt added 2 commits August 23, 2019 22:16

fix(docs/api): Fix changes suggested in the review

d962528

feat(docs/api): Add endpoints for swag items

a8c5e4c

* Add key "item" in model "Swag" as a foreign key to hold "SwagItems". These items will behave like choices for creating the swag.

gutsytechster reviewed Aug 30, 2019

View reviewed changes

Apply changes suggested in the review

f66d3ce

CuriousLearner requested changes Sep 7, 2019

View reviewed changes

	PATCH /api/swag/:swag_id (requires authentication)
	PATCH /api/swag/:id (requires authentication)

	- For `lightning_talk`, duration will be fixed like `00:05:00`, etc. which would be configuration through settings.
	- For `lightning_talk`, the duration will be fixed like `00:05:00`, etc. which would be configurable through settings.

Conversation

GeekyShacklebolt commented Aug 22, 2019

Uh oh!

CuriousLearner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GeekyShacklebolt Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CuriousLearner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GeekyShacklebolt Aug 23, 2019 •

edited

Loading