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

Opentask view Support #1279

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Opentask view Support #1279

wants to merge 17 commits into from

Conversation

mebitek
Copy link

@mebitek mebitek commented Dec 19, 2022

I added Opentask support in read-only mode:
current features

show/hide task calendars
view task on day, week, month and agenda view
view task detail with markdown support
support Opentask
share task as VTODO
add option to show/hide non-visible calendars in main preferences list
widget show tasks

@mebitek
Copy link
Author

mebitek commented Dec 19, 2022

@jspricke I think that now is better

Copy link
Member

@jspricke jspricke left a comment

Choose a reason for hiding this comment

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

stopped after finding two problems..

Please don't open new PRs but keep this one now.

app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
app/src/main/java/com/android/calendar/Event.java Outdated Show resolved Hide resolved
@mebitek
Copy link
Author

mebitek commented Dec 19, 2022

@jspricke maybe I not understanding the situation but if I compare my branch with master etar I get this:
master...mebitek:Etar-Calendar:master

could you please help me to understand?

@jspricke
Copy link
Member

You undo the changes in your last commit: c68bf76

Instead you should use git rebase -i to squash those changes into the original commits.

@mebitek mebitek force-pushed the master branch 5 times, most recently from e2a5760 to ce843fe Compare December 19, 2022 18:31
@jspricke
Copy link
Member

Can you please finish your commits before pushing, as every push produces a mail for me ;).

@mebitek
Copy link
Author

mebitek commented Dec 19, 2022

@jspricke sorry but I cannot understand how to solve this situation. I think that I solve as
https://github.com/Etar-Group/Etar-Calendar/compare/master...mebitek:Etar-Calendar:master?expand=1 says.
if not I need help

@jspricke
Copy link
Member

See your list of commits here: https://github.com/Etar-Group/Etar-Calendar/pull/1279/commits
You have split your PR in individual commits, which is a good thing as it helps us to understand your changes. You ca, show the individual commits, for example: 4cfce30 and as you can see, it still contains unrelated changes that are undone in a later commit. This makes it hard to review because we need to find the relevant changes and keep in mind to check if the unrelated changes are fixed later. Also it makes it hard to understand whenever someone has to go back in the commit history to find out from where a change came. So instead you want to go back to fix that commit to contain only the relevant changes. git rebase -i does that, you can set a commit to edit, undo the changes and then go to the next one. Here are some articles that may be useful:
https://about.gitlab.com/blog/2018/06/07/keeping-git-commit-history-clean/
https://medium.com/@catalinaturlea/clean-git-history-a-step-by-step-guide-eefc0ad8696d

@Gitsaibot
Copy link
Contributor

All the git stuff is something for @jspricke ;). I'll try to test this over the holidays when I get a chance.

Copy link
Member

@jspricke jspricke left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the commits, looks much better now.
I've left some general comments, but did not look into the patches deeply.

For the requested changes you can use git add -i and git commit --fixup <sha1> to add fixup commits that can be autosquashed later.

@Gitsaibot
Copy link
Contributor

Gitsaibot commented Jan 5, 2023

If I click on "View Tasks" in Agenda View Etar crashes(Failed to find provider info for org.dmfs.tasks)
And I get a lot of "E/CalEvent: buildEventsFromCursor: null cursor or null events list!" messages.

@mebitek
Copy link
Author

mebitek commented Jan 6, 2023

Are opentasks installed? I think I miss the check on agenda view?

@Gitsaibot
Copy link
Contributor

Yes I haven't installed it yet.

@mebitek
Copy link
Author

mebitek commented Jan 11, 2023

@Gitsaibot fixed. added a check to see if opentasks has been installed. if not the menu "view tasks" is not visible

@Gitsaibot
Copy link
Contributor

When opentask is not installed i still get these debug messages:

debug E/ActivityThread: Failed to find provider info for org.dmfs.tasks
2023-01-17 18:06:01.425 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:02.393 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:03.996 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:04.985 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:06.176 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:07.642 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:08.611 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:10.254 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:11.723 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:12.816 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!
2023-01-17 18:06:15.706 2172-2172/ws.xsoh.etar.debug E/CalEvent: buildEventsFromCursor: null cursor or null events list!

@mebitek
Copy link
Author

mebitek commented Jan 18, 2023

@Gitsaibot fixed

README.md Outdated Show resolved Hide resolved
@Gitsaibot
Copy link
Contributor

I still get the messages when I switch between calendar views.

@mebitek
Copy link
Author

mebitek commented Jan 19, 2023

@Gitsaibot now is fixed

Copy link
Member

@jspricke jspricke left a comment

Choose a reason for hiding this comment

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

I did some clean up of the commits, please update your local branch before doing new changes, for example by:

git fetch origin
get reset --hard origin/master

@@ -147,7 +147,7 @@ protected void onCreate(Bundle icicle) {
if (mInfoFragment == null) {
FragmentManager fragmentManager = getFragmentManager();
FragmentTransaction ft = fragmentManager.beginTransaction();
mInfoFragment = new EventInfoFragment(this, mEventId, mStartMillis, mEndMillis,
mInfoFragment = new EventInfoFragment(this, intent.getData(), mStartMillis, mEndMillis,
Copy link
Member

Choose a reason for hiding this comment

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

are you sure intent is never null here?

Comment on lines +217 to 237
if (isTask) {
val taskUri =
ContentUris.withAppendedId(DmfsOpenTasksContract.TaskLists.PROVIDER_URI, calendarId)
contentResolver.query(taskUri, ACCOUNT_PROJECTION, null, null, null)?.use {
if (it.moveToFirst()) {
val accountName = it.getString(PROJECTION_ACCOUNT_INDEX_NAME)
val accountType = it.getString(PROJECTION_ACCOUNT_INDEX_TYPE)
return Account(accountName, accountType)
}
}
} else {
val calendarUri =
ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, calendarId)
contentResolver.query(calendarUri, ACCOUNT_PROJECTION, null, null, null)?.use {
if (it.moveToFirst()) {
val accountName = it.getString(PROJECTION_ACCOUNT_INDEX_NAME)
val accountType = it.getString(PROJECTION_ACCOUNT_INDEX_TYPE)
return Account(accountName, accountType)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (isTask) {
val taskUri =
ContentUris.withAppendedId(DmfsOpenTasksContract.TaskLists.PROVIDER_URI, calendarId)
contentResolver.query(taskUri, ACCOUNT_PROJECTION, null, null, null)?.use {
if (it.moveToFirst()) {
val accountName = it.getString(PROJECTION_ACCOUNT_INDEX_NAME)
val accountType = it.getString(PROJECTION_ACCOUNT_INDEX_TYPE)
return Account(accountName, accountType)
}
}
} else {
val calendarUri =
ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, calendarId)
contentResolver.query(calendarUri, ACCOUNT_PROJECTION, null, null, null)?.use {
if (it.moveToFirst()) {
val accountName = it.getString(PROJECTION_ACCOUNT_INDEX_NAME)
val accountType = it.getString(PROJECTION_ACCOUNT_INDEX_TYPE)
return Account(accountName, accountType)
}
}
}
if (isTask) {
val uri =
ContentUris.withAppendedId(DmfsOpenTasksContract.TaskLists.PROVIDER_URI, calendarId)
} else {
val uri =
ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, calendarId)
}
contentResolver.query(uri, ACCOUNT_PROJECTION, null, null, null)?.use {
if (it.moveToFirst()) {
val accountName = it.getString(PROJECTION_ACCOUNT_INDEX_NAME)
val accountType = it.getString(PROJECTION_ACCOUNT_INDEX_TYPE)
return Account(accountName, accountType)
}
}
}

// populate the day buckets that this event falls into
int from = Math.max(startDay, mTodayJulianDay);
int to = Math.min(endDay, mMaxJulianDay);
int from = Math.max(Time.getJulianDay(mEventInfo.start, new Time().getGmtOffset()), mTodayJulianDay);
Copy link
Member

Choose a reason for hiding this comment

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

can you give some explanation here?

Copy link
Author

Choose a reason for hiding this comment

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

same as before. tasks has no begin time, so it will be recalcualted

Copy link
Member

Choose a reason for hiding this comment

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

can you give some more explanation? Why do you need a loop? Why an extra method? Why change the computation?

Copy link
Author

Choose a reason for hiding this comment

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

cause tasks comes from different objects than events, so I need to calculate dates

Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to compute them in the object instead of different times in the code?

Copy link
Member

Choose a reason for hiding this comment

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

@mebitek just a friendly pint. Do you have any comment here? I think I would be fine with the current state but maybe this would simplify the code a bit.

Copy link
Author

Choose a reason for hiding this comment

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

@jspricke sorry but I was a little busy these days. if it will be possiible to leave as it is for now should be better for me

// populate the day buckets that this event falls into
int from = Math.max(startDay, mTodayJulianDay);
int to = Math.min(endDay, mMaxJulianDay);
int from = Math.max(Time.getJulianDay(mEventInfo.start, new Time().getGmtOffset()), mTodayJulianDay);
Copy link
Member

Choose a reason for hiding this comment

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

can you give some more explanation? Why do you need a loop? Why an extra method? Why change the computation?

if (it.moveToFirst()) {
return it.getString(it.getColumnIndex(databaseKey))
}
}
return defValue
}
private fun getValues(key: String?) : Map<String, Any> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing new line.

Copy link
Author

Choose a reason for hiding this comment

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

the loop was already present. I just move variables to meet day calculation

Copy link
Author

Choose a reason for hiding this comment

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

new line fixed

Copy link
Member

@jspricke jspricke Apr 11, 2023

Choose a reason for hiding this comment

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

sorry to be really picky but you added a new line and four spaces, please remove them.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

sorry I do not understand

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

if I remeber well it's cause tasks has different time enconding in front of events, so I need to recalculate

@jspricke
Copy link
Member

I gave this a quick try and it shows tasks nicely but clicking on a task shows an empty loading screen and editing one loads a different event. Can you implement some workaround there?

@mebitek
Copy link
Author

mebitek commented Apr 27, 2023

just tested now and seems to work on my android, can you provide more details?

@jspricke
Copy link
Member

Not sure what to provide, what happens when you select a task from the calendar?

@mebitek
Copy link
Author

mebitek commented Apr 27, 2023

Not sure what to provide, what happens when you select a task from the calendar?

it should open the task details, in readonly mode

@mebitek
Copy link
Author

mebitek commented Jun 22, 2023

@jspricke @Gitsaibot any news about this pull request?

@tamas646
Copy link

tamas646 commented Oct 5, 2023

I gave this a quick try and it shows tasks nicely but clicking on a task shows an empty loading screen and editing one loads a different event. Can you implement some workaround there?

@mebitek I experience the same behaviour. When I select a task, it shows an empty loading screen.

This was referenced Oct 5, 2023
@cboehm-it
Copy link

Hey that feature seems to be very cool. Is there already a release planned for it? 🙂

@tamas646
Copy link

@immertroll There are still some issues/incomplete parts with this feature which is the reason why this pull request has not been merged yet.
You can build some debug apk for yourself from mebitek's repository (https://github.com/mebitek/Etar-Calendar) using Android Studio (as I did). This way you can use the feature but as I said there are some incomplete parts in it (and of course it's based on an older version of Etar-Calendar with no upgrading possibility).

It would be great if someone would complete the feature and therefore it could be added to Etar.

@estux
Copy link

estux commented Jun 6, 2024

@immertroll There are still some issues/incomplete parts with this feature which is the reason why this pull request has not been merged yet. You can build some debug apk for yourself from mebitek's repository (https://github.com/mebitek/Etar-Calendar) using Android Studio (as I did). This way you can use the feature but as I said there are some incomplete parts in it (and of course it's based on an older version of Etar-Calendar with no upgrading possibility).

It would be great if someone would complete the feature and therefore it could be added to Etar.

It would be great to also add support for Tasks app as OpenTask is not developed anymore and it's becoming outdated.
Thanks for the work done so far!

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.

6 participants