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

Simplify chainable Resource.Promise promises #193

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Jan 27, 2023

Use Array.map() to simplify the code that was generated by CoffeeScript 2.7 for Resource.Promise. This should make it easier to read (and follow?)

zooniverse/json-api-client#84 fixed zooniverse/Panoptes-Front-End#6380 and zooniverse/Panoptes-Front-End#6381 but the generated JavaScript is hard to read. This replaces it with a map of API resources => resource promises.

@eatyourgreens
Copy link
Contributor Author

The original bug broke chained calls to client.type().get(), which returns an instance of Resource.Promise.

// get subjects for a subject set
apiClient
  .type('set_member_subjects')
  .get({ subject_set_id })
  .get('subject')
  .then(subjects => console.log(subjects));

// edit an existing Talk comment
talkClient
  .type('comments')
  .get({ id: commentId })
  .update(newComment)
  .save();

The fix involved declaring delete, get, save and update methods that all call the corresponding method on the original resource, which then allows resources to be chained.

@eatyourgreens
Copy link
Contributor Author

Now that #192 is merged, this could also have some tests.

Use `Array.map()` to simplify the code that was generated by CoffeeScript 2.7 for `Resource.Promise`. This should make it easier to read (and follow?)
Copy link
Contributor

@mcbouslog mcbouslog left a comment

Choose a reason for hiding this comment

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

LGTM, certainly more readable 👍 !

Tested locally with PFE installed with a package.json updated to "panoptes-client": "zooniverse/panoptes-javascript-client#chainable-resource-promise".

Locally I signed in/out, made a few classifications, in the lab I created a subject set and workflow ✅ .

@eatyourgreens eatyourgreens merged commit da4cfbe into master Feb 3, 2023
@eatyourgreens eatyourgreens deleted the chainable-resource-promise branch February 3, 2023 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to edit my own Talk posts
2 participants