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

Add RowData to create recurring tasks. Implements #460 #604

Merged
merged 3 commits into from
Jan 3, 2018

Conversation

dmfs
Copy link
Owner

@dmfs dmfs commented Dec 30, 2017

This certainly needs to be revisited, but it should be ok for now when it's mostly required to write unit tests.

This certainly needs to be revisited, but it should be ok for now when it's mostly required to write unit tests.
@dmfs dmfs requested a review from lemonboston December 30, 2017 13:57
@Override
public ContentProviderOperation.Builder updatedBuilder(@NonNull TransactionContext transactionContext, @NonNull ContentProviderOperation.Builder builder)
{
String value = TextUtils.join(",",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not extract the code duplication between ExDatesTaskData and RDatesTaskData?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Right, I'll take care of it.

/**
* {@link RowData} for tasks with EXDATEs
* <p>
* TODO: how to make sure this is only ever used with tasks having a start and/or due date?
Copy link
Contributor

Choose a reason for hiding this comment

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

Should that be the responsibility of the provider? To ignore these fields in the input if there is no start/due date.

Copy link
Owner Author

Choose a reason for hiding this comment

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

No, a content provider client must send valid data. I want to reduce this kind of fault tolerance to the bare minimum. The provider is supposed to throw if it receives recurrence data for tasks without start or due (at present it doesn't).

@dmfs
Copy link
Owner Author

dmfs commented Jan 2, 2018

@lemonboston ready for re-review.

Copy link
Contributor

@lemonboston lemonboston left a comment

Choose a reason for hiding this comment

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

Looks good.

*
* @author Marten Gajda
*/
public final class DateTimeListTaskData implements RowData<TaskContract.Tasks>
Copy link
Contributor

Choose a reason for hiding this comment

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

Making it package-private perhaps?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I chose to generalize it and moved it to its own package. It might be useful in other places (although I don't see any atm). Check again.

Copy link
Contributor

@lemonboston lemonboston left a comment

Choose a reason for hiding this comment

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

Looks good.

@dmfs dmfs merged commit 69d47da into master Jan 3, 2018
@dmfs dmfs deleted the stories/460-recuring-row-data branch January 3, 2018 18:35
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.

2 participants