X Tutup
The Wayback Machine - https://web.archive.org/web/20200903040712/https://github.com/redux-orm/redux-orm/issues/291
Skip to content

/ redux-orm

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

Jump to bottom

create and remove methods on backwardsManyToOneDescriptor #291

Open
videni opened this issue Sep 29, 2019 · 7 comments
Open

create and remove methods on backwardsManyToOneDescriptor #291

videni opened this issue Sep 29, 2019 · 7 comments
Labels
good first issue layer: Fields layer: QuerySet type: Enhancement

Comments

@videni
Copy link

@videni videni commented Sep 29, 2019 •
edited

what is prefer way to add relation and remove relation? I found example as below, but I did't find any document about the two methods.

 case 'ADD_AUTHOR_TO_BOOK':
        Book.withId(action.payload.bookId).authors.add(action.payload.author);  // add new entity ,  add relation?  this doesn't work.
        break;
    case 'REMOVE_AUTHOR_FROM_BOOK':
        Book.withId(action.payload.bookId).authors.remove(action.payload.authorId);  // remove entity and relation?
        break;

ReportConfig vs ReportConfigGroup

one-to-many

I also tried

add relation,

 ReportConfig.withId(action.payload.configId).groups.create(action.payload.data);  doesn't work.

ReportConfig.withId(action.payload.configId).groups.add(action.payload.data); //  doesn't work.

delete relation, it will remove the group entity and the relation. works as expected.

 case 'reportConfig/deleteGroupSuccess': {
          ReportConfigGroup.withId(action.payload.groupId).delete();
          break;
        }
@haveyaseen
Copy link
Member

@haveyaseen haveyaseen commented Sep 29, 2019

As noted in #275 (comment) add, remove and clear are only available for many-to-many QuerySets. For one-to-many relationships the dependent model instance simply has to be created with the correct foreign key. reportConfig.groups.delete() should correctly delete all related groups.

We can actually add add and remove to backwardsManyToOneDescriptor. I can definitely see why this would be useful. This would increase the amount of API calls we need to maintain. Should we find a better approach to normalization we can remove them later on (in v1 or v2).
@haveyaseen haveyaseen changed the title how to add relation? create and add methods on backwardsManyToOneDescriptor Sep 29, 2019
@lexcorp16
Copy link

@lexcorp16 lexcorp16 commented Oct 1, 2019

Hi, @haveyaseen can you point me in the right direction as to what needs to be done? thanks.
@haveyaseen
Copy link
Member

@haveyaseen haveyaseen commented Oct 1, 2019

Hi @lexcorp16, that would be awesome if you could help us out! Essentially the backwardsManyToOneDescriptor's get() method needs to transform the QuerySet that it returns.

redux-orm/src/descriptors.js

Lines 115 to 132 in 5738fe4

function backwardsManyToOneDescriptor(declaredFieldName, declaredFromModelName) {
return {
get() {
const {
session: {
[declaredFromModelName]: DeclaredFromModel,
},
} = this.getClass();
return DeclaredFromModel.filter({
[declaredFieldName]: this.getId(),
});
},
set() {
throw new Error('Can\'t mutate a reverse many-to-one relation.');
},
};
}

Instead of directly returning DeclaredFromModel.filter(…) the result should be stored in a variable, and additional methods add and remove should be monkey-patched. clear is only needed for many-to-many descriptors and should not be added. See the manyToManyDescriptor for an example of what the get function needs to look like:

redux-orm/src/descriptors.js

Lines 189 to 273 in 5738fe4

const qs = OtherModel.filter((otherModelInstance) => (
referencedOtherIds.has(
otherModelInstance[OtherModel.idAttribute]
)
));
/**
* Allows adding OtherModel instances to be referenced by the current instance.
*
* E.g. Book.first().authors.add(1, 2) would add the authors with IDs 1 and 2
* to the first book's list of referenced authors.
*
* @return undefined
*/
qs.add = function add(...entities) {
const idsToAdd = new Set(
entities.map(normalizeEntity)
);
const existingQs = throughQs.filter((through) => (
idsToAdd.has(through[otherReferencingField])
));
if (existingQs.exists()) {
const existingIds = existingQs
.toRefArray()
.map((through) => through[otherReferencingField]);
throw new Error(`Tried to add already existing ${OtherModel.modelName} id(s) ${existingIds} to the ${ThisModel.modelName} instance with id ${thisId}`);
}
idsToAdd.forEach((id) => {
ThroughModel.create({
[otherReferencingField]: id,
[thisReferencingField]: thisId,
});
});
};
/**
* Removes references to all OtherModel instances from the current model.
*
* E.g. Book.first().authors.clear() would cause the first book's list
* of referenced authors to become empty.
*
* @return undefined
*/
qs.clear = function clear() {
throughQs.delete();
};
/**
* Removes references to all passed OtherModel instances from the current model.
*
* E.g. Book.first().authors.remove(1, 2) would cause the authors with
* IDs 1 and 2 to no longer be referenced by the first book.
*
* @return undefined
*/
qs.remove = function remove(...entities) {
const idsToRemove = new Set(
entities.map(normalizeEntity)
);
const entitiesToDelete = throughQs.filter(
(through) => idsToRemove.has(through[otherReferencingField])
);
if (entitiesToDelete.count() !== idsToRemove.size) {
// Tried deleting non-existing entities.
const entitiesToDeleteIds = entitiesToDelete
.toRefArray()
.map((through) => through[otherReferencingField]);
const unexistingIds = [...idsToRemove].filter(
(id) => !entitiesToDeleteIds.includes(id)
);
throw new Error(`Tried to delete non-existing ${OtherModel.modelName} id(s) ${unexistingIds} from the ${ThisModel.modelName} instance with id ${thisId}`);
}
entitiesToDelete.delete();
};
return qs;

@haveyaseen
Copy link
Member

@haveyaseen haveyaseen commented Oct 1, 2019 •
edited

@lexcorp16 You can use the DeclaredFromModel class to .create() model instances and qs.filter(…).delete() to delete a certain subset of related entities.

Now that I think about it, the add function should probably be called create and it should receive a list of entities where each entity is changed to contain the current model's primary key (this.getId()) at the foreign key position of the related model, i.e. each newly created model automatically gets its declaredFieldName field set to this.getId().

It may be best to release a batch version of Model.create at the same time (and also a static Model.delete function) and re-use it for this exact purpose. See also #187. But if you can do it within the descriptor it would be great, too.
@haveyaseen haveyaseen changed the title create and add methods on backwardsManyToOneDescriptor create and remove methods on backwardsManyToOneDescriptor Oct 1, 2019
@haveyaseen haveyaseen mentioned this issue Oct 1, 2019
Open
@KaushikIyer16
Copy link

@KaushikIyer16 KaushikIyer16 commented Oct 25, 2019

hi is anyone still actively working on this ? if not can i take a crack at this ? thank you
@haveyaseen
Copy link
Member

@haveyaseen haveyaseen commented Oct 26, 2019

@KaushikIyer16 Sure! Looks like it's safe to assume no one else is working on this.
@haveyaseen haveyaseen mentioned this issue Jan 3, 2020
Closed
@ManuelPortero
Copy link

@ManuelPortero ManuelPortero commented Aug 20, 2020

I would like to work on this too. May be work with something cause i'm new in redux. Can I join ?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue layer: Fields layer: QuerySet type: Enhancement
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
@videni @haveyaseen @KaushikIyer16 @lexcorp16 @ManuelPortero
You can’t perform that action at this time.
X Tutup