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

feat: docs, tests and fixes for create/update/deleteRecord builders #8849

Merged
merged 16 commits into from
Sep 17, 2023

Conversation

Baltazore
Copy link
Collaborator

Documentation for rest of the builder functions

This PR aims to cover create-record / update-record / delete-record documentation

Should be added to json-api / rest / active-record packages

@Baltazore Baltazore force-pushed the add-save-record-docs branch 3 times, most recently from 6b6fb7d to 9d8be6c Compare September 15, 2023 20:16
export function createRecord(record: unknown, options: ConstrainedRequestOptions = {}): CreateRequestOptions {
const identifier = recordIdentifierFor(record);
assert(`Expected to be given a record instance`, identifier);
assert(`Cannot delete a record that does not have an associated type and id.`, isExisting(identifier));
// TODO: seems to be wrong assert here, at least message is confusing
// assert(`Cannot delete a record that does not have an associated type and id.`, isExisting(identifier));

Choose a reason for hiding this comment

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

+1 to wrong assertion, you should be able to create a record without an identifier.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leepfrog not exactly. You do need an identifier, the identifier does not need an id.

Choose a reason for hiding this comment

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

Whoops! Yes, agree!

@runspired runspired added 🎯 canary PR is targeting canary (default) 🏷️ test This PR primarily adds tests for a feature 🏷️ feat This PR introduces a new feature labels Sep 17, 2023
@runspired runspired changed the title WIP: add create-record/update-record/delete-record builder docs feat: docs, tests and fixes for create/update/deleteRecord builders Sep 17, 2023
// Handle testing feature flags
if (QUnit.urlParams.enableoptionalfeatures) {
window.EmberDataENV = { ENABLE_OPTIONAL_FEATURES: true };
}

configureAsserts();

setApplication(Application.create(config.APP));
Copy link
Contributor

Choose a reason for hiding this comment

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

in follow up we should eliminate this and the new app files in favor of in-test-file registration

@runspired runspired merged commit 7d4f65c into emberjs:main Sep 17, 2023
21 checks passed
@runspired runspired added 🎯 release PR should be backported to release 🎯 lts The PR should be backported to the most recent LTS lts-4-12 Long Term LTS Maintenance and removed 🎯 release PR should be backported to release labels Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) 🎯 lts The PR should be backported to the most recent LTS 🏷️ feat This PR introduces a new feature 🏷️ test This PR primarily adds tests for a feature lts-4-12 Long Term LTS Maintenance
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants