Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

Make API consistent for creating instances #62

Closed
mnugter opened this issue Jul 5, 2019 · 3 comments
Closed

Make API consistent for creating instances #62

mnugter opened this issue Jul 5, 2019 · 3 comments
Assignees
Milestone

Comments

@mnugter
Copy link
Collaborator

mnugter commented Jul 5, 2019

Repositories are now created via the repository manager. This should change to Repository.create and should use a default enabled decorator for the manager.

Same goes for HttpConnector, it should have a decorateable create factory method.

@mnugter mnugter added this to the 1.1 milestone Jul 5, 2019
@mnugter
Copy link
Collaborator Author

mnugter commented Jul 5, 2019

repositoryManager actually has a good reason to be there as the ChangeSet / Task structure relies on it heavily. It should not be optional/unsetable (as a decorator is) and therefore it should stay intact.

Added support for both for a factory+decorators though.

@mnugter mnugter self-assigned this Jul 5, 2019
@ekampp
Copy link

ekampp commented Jul 10, 2019

Hey. Just tried implementing this library today.

Not sure if this is the best place to put this comment, but it relates to creating resources.

When I create a resource using this part of the quick-start guide I see that the resulting URL that's called on the API server contains /null at the end.

import Resource from '@hyral/core/lib/Resource/Resource';
const authenticateResource = repositoryManager.createRepository(connector, "authenticate");
const authenticate = Resource.create(null, "authenticate", {
  email: "my@email.com",
  password: "password",
});

This results in a GET request to /authenticate/null on my server.

Two things stand out as wrong here:

  1. Creating a resource should be a POST request, not a GET request?
  2. Using null as the ID should not actually put null in the URL.

Am I doing something wrong?

Thanks,
Emil

@mnugter
Copy link
Collaborator Author

mnugter commented Jul 11, 2019

Hi @ekampp ,

Awesome that you're using the library! I'm glad to help you out. Maybe a new issue with the label question whould be better as it would keep the current issues clean.

I'm curious as to where a GET request would be triggered. It should not be triggered by the code above. I will try to reproduce it and if I'm able I will look into fixing it. If it actually does trigger a request it might have something to do with the lazy-loading decorator and an incorrect request to the data. To some up, it should certainly not result in a GET request to /authenticate/null.

The code here is not wrong but it is incomplete if you want to create the resource on the server. For a change to be sent to the server you need to create a ChangeSet, persist the new resource and execute the changeset:

`
// Create ChangeSet to queue changes.
const changeSet = ChangeSet.create();

// Use persistChange to only commit changes to the passed resource.
changeSet.persistResource(authenticate);

// Send all queued changes to the backend.
changeSet.execute().then(() => {
// The authenticate instances will be updated and now have an ID.
console.log('resource created!');
});
`

I hope this information helps!

barthje added a commit that referenced this issue Jul 12, 2019
@mnugter mnugter closed this as completed Jul 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants