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

Feature: Extend operation options #433

Merged
merged 14 commits into from
Jan 30, 2018
Merged

Conversation

3lvis
Copy link
Owner

@3lvis 3lvis commented Oct 24, 2017

Adds the ability to tell Sync to insert, remove or updates relationships.

  • One-to-many
  • Many-to-many
  • One-to-one

@3lvis 3lvis changed the title [WIP] Feature: Extend operation options Feature: Extend operation options Oct 30, 2017
@aamir-nazir
Copy link
Collaborator

@3lvis when we are planing to merge this feature in master?

@3lvis
Copy link
Owner Author

3lvis commented Jan 6, 2018

@aamir-nazir Need to get the other operations in place, I'm happy to receive some help :)

@aamir-nazir
Copy link
Collaborator

@3lvis yes sure, I am up for to do some of the tasks.

@3lvis
Copy link
Owner Author

3lvis commented Jan 6, 2018

@aamir-nazir Added you as a collaborator https://github.com/3lvis/Sync/invitations

I think a good place to start would be to understand the unit tests added in this PR and add more unit tests for the remaining cases. Right now we only have tests for one-to-many

  • Many-to-many
  • One-to-one

@aamir-nazir
Copy link
Collaborator

@3lvis do we seeking more Test Cases? I can be able to implement some of them on weekend.

@3lvis
Copy link
Owner Author

3lvis commented Jan 13, 2018

@aamir-nazir For Many-to-many and One-to-one I've added a JSON and a Core Data model but the unit tests were just copied from One-to-many, so we still need tests for many-to-many and one-to-one. Thanks for your help!

@batjo
Copy link
Collaborator

batjo commented Jan 29, 2018

@3lvis I don't think this needs to be implemented for one-to-one right? I can see the use case for one-to-many and many-to-many. But for one-to-one you would always like to insert/update/delete the changes in my opinion

@batjo
Copy link
Collaborator

batjo commented Jan 30, 2018

I just fixed the tests for the many-to-many relationships. Code was already working, it was just that the tests where wrong. Still I don't see the use case for a one-to-one relationship. The use case that has been described by others (where the server only sends the last x messages instead of all messages) makes sense to me. But for a one-to-one relationship it doesn't make sense since the json will always contain the object. So I think this can be merged

@3lvis 3lvis merged commit e0248f9 into master Jan 30, 2018
@3lvis 3lvis deleted the feature/extend-operation-options branch January 30, 2018 23:18
@3lvis
Copy link
Owner Author

3lvis commented Jan 30, 2018

I agree @batjo! Is better to get it out and get feedback as well. Thanks for your help!

@3lvis 3lvis mentioned this pull request Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants