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 agendaupdate subcommand #553

Merged
merged 18 commits into from
Mar 3, 2021

Conversation

michaelmhoffman
Copy link
Collaborator

@michaelmhoffman michaelmhoffman commented Aug 2, 2020

Add the agendaupdate subcommand, the minimum viable product for #550.

Refactor gcalcli.gcal.GoogleCalendarInterface._tsv(), replacing the long list of if statements with a modular design using a series of gcalcli.details.Handler classes.

My plan is to keep making additions to the michaelmhoffman/issue-550 branch, pushing from my local machine when I have changes that implement each of the checklist items in #550. I test locally with Python 3.5-Python 3.8. Please feel free to review and apply now or when I'm done with the whole set.

Refactor `gcalcli.gcal.GoogleCalendarInterface._tsv()`, replacing the
long list of if statements with a modular design using a series of
`gcalcli.details.Handler` classes.

Each `Handler` represents the set of event properties that are
captured by each item of `gcalcli.argparsers.DETAILS`. So far only the
`header` class attribute and the `get()` methods are implemented. This
is what is necessary for TSV output.

Preparatory work for insanum#550.
These are mostly overriding pure virtual classes and adding a
docstring to each one would be superfluous and make the code harder to
read.
Set `gcalcli.argparsers.DETAILS` to a superset of
`gcalcli.details.HANDLERS.keys()` to avoid duplication.

Preparatory work for insanum#550.
Can't use a property on a classmethod without making a custom
metaclass. Without that, the property is only enacted upon class
instantiation, which we aren't doing here.

A custom metaclass is too magic, and thus far we have avoided
instantiating what would be singleton objects.

Also changed `SimpleSingleColumnHandler._get()` to use `cls.header[0]`
instead of `cls._key` which is removed.
BREAKING CHANGE: having a header row will break any software that
assumes that the output of `gcalcli agenda --tsv` doesn't have one.

Also slightly refactored the output printing to use print() like in
most parts of the code.

Partially implements insanum#550.
argparsers.py, cli.py: add `agendaupdate` subcommand
gcal.py: add `AgendaUpdate()`
details.py:
- `Title`, `SimpleSingleFieldHandler`: add `patch()`
- add `FIELD_HANDLERS`

Minimum viable product for insanum#550.
@michaelmhoffman michaelmhoffman changed the title Move _tsv() implementation to gcalcli.details Add agendaupdate subcommand Aug 4, 2020
This will enable `Handler`s with multiple fields to work for patching.
Necessary for `Time.patch()` and `Conference.patch()`

Also introduce `SingleFieldHandler.patch(cls,` ...`)` which dispatches to
`cls._patch()`.

Preparatory for further implementation of insanum#550.
Round trips don't work for all day events. The current output format
is to present them as events starting and ending at midnight of the
same day. But this keeps us from having events starting and ending at
midnight of the same day.

In the next commit I plan to change the output for all day events so
the time fields are blank.

Also:

- change signature of `Handler.patch()` to include `cal`, which is necessary to extract the timezone
- change default of `Handler.fieldnames` from `None` to `[]` to silence a mypy warning.

Partially implements insanum#550.
…ll_day()

This enables its use by details.py.
BREAKING CHANGE: this changes the previous behavior which was to
depict all-day events as starting and ending at midnight.
…es to get

The new `details.URL_PROPS` constant `OrderedDict` will also work for
use in `Url.patch()`, which should be implemented in the next commit.
This will allow automated handling of two property/fieldname pairs for
this `Handler`.
Both these fields are read-only.

If html_link is in the input, always fail.
If hangout_link is in the input, check against the current value. If they don't match, fail.
This treats calendar as a readonly field. Actually moving calendar
would be a bit more involved, and would necessitate:
- changing agendaupdate from its current one-calendar-only restriction
- using the events().move() method to do the move. It's not done by events().patch()

That can be a future feature.

Partially implements insanum#550.
@michaelmhoffman
Copy link
Collaborator Author

@jcrowgey This pull request fully implements #550, passes the tox tests, and is ready for review.



def is_all_day(event):
# XXX: currently gcalcli represents all-day events as those that both begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point! Is there a field in the API event structure which disambiguates these cases?

if handler in HANDLERS_READONLY)

_DETAILS_WITHOUT_HANDLERS = ['length', 'reminders', 'attendees',
'attachments', 'end']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to incorporate the 'end' detail into the Time handler?

Copy link
Collaborator

@jcrowgey jcrowgey left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for this work!

@insanum
Copy link
Owner

insanum commented Sep 30, 2020

@michaelmhoffman would you be interested in helping us mange the gcalcli project and have commit access? Your contributions have been meaningful and I'm sure you can help guide gcalcli in the right direction.

@michaelmhoffman
Copy link
Collaborator Author

michaelmhoffman commented Oct 3, 2020 via email

@michaelmhoffman
Copy link
Collaborator Author

Should I use a merge commit, squash and merge, or rebase and merge?

@michaelmhoffman
Copy link
Collaborator Author

Looks like squash was what was used before.

@michaelmhoffman michaelmhoffman merged commit bc00f41 into insanum:master Mar 3, 2021
@michaelmhoffman michaelmhoffman deleted the issue-550 branch March 3, 2021 16:01
kda pushed a commit to kda/gcalcli that referenced this pull request May 12, 2023
* feat: add agendaupdate for calendar

This treats calendar as a readonly field. Actually moving calendar
would be a bit more involved, and would necessitate:
- changing agendaupdate from its current one-calendar-only restriction
- using the events().move() method to do the move. It's not done by events().patch()

That can be a future feature.

Partially implements insanum#550.

* feat!: add header row for `gcalcli agenda --tsv`

BREAKING CHANGE: having a header row will break any software that
assumes that the output of `gcalcli agenda --tsv` doesn't have one.

Also slightly refactored the output printing to use print() like in
most parts of the code.

* feat!: gcal agenda --tsv prints empty time fields for all-day events

BREAKING CHANGE: this changes the previous behavior which was to
depict all-day events as starting and ending at midnight.

* feat: add `--detail id` for `gcalcli agenda --tsv`

* feat: add agendaupdate for title, id, location, description

argparsers.py, cli.py: add `agendaupdate` subcommand
gcal.py: add `AgendaUpdate()`
details.py:
- `Title`, `SimpleSingleFieldHandler`: add `patch()`
- add `FIELD_HANDLERS`

* feat: add agendaupdate for time

Round trips don't work for all day events. The current output format
is to present them as events starting and ending at midnight of the
same day. But this keeps us from having events starting and ending at
midnight of the same day.

Also:

- change signature of `Handler.patch()` to include `cal`, which is necessary to extract the timezone
- change default of `Handler.fieldnames` from `None` to `[]` to silence a mypy warning.

* feat: add agendaupdate for url

Both these fields are read-only.

If html_link is in the input, always fail.
If hangout_link is in the input, check against the current value. If they don't match, fail.

* feat: add agendaupdate for conference

Partially implements insanum#522.

* refactor: move _tsv() implementation to gcalcli.details

Refactor `gcalcli.gcal.GoogleCalendarInterface._tsv()`, replacing the
long list of if statements with a modular design using a series of
`gcalcli.details.Handler` classes.

Each `Handler` represents the set of event properties that are
captured by each item of `gcalcli.argparsers.DETAILS`. So far only the
`header` class attribute and the `get()` methods are implemented. This
is what is necessary for TSV output.

* fix: change SimpleSingleColumnHandler.header to a simple list

Can't use a property on a classmethod without making a custom
metaclass. Without that, the property is only enacted upon class
instantiation, which we aren't doing here.

A custom metaclass is too magic, and thus far we have avoided
instantiating what would be singleton objects.

Also changed `SimpleSingleColumnHandler._get()` to use `cls.header[0]`
instead of `cls._key` which is removed.

* ci: eliminate flake8-docstrings method docstring checks for details.py

These are mostly overriding pure virtual classes and adding a
docstring to each one would be superfluous and make the code harder to
read.

* refactor: derive `argparsers.DETAILS` from `details.HANDLERS`

Set `gcalcli.argparsers.DETAILS` to a superset of
`gcalcli.details.HANDLERS.keys()` to avoid duplication.

* refactor: change `header` to `fieldnames` and `column` to `field`

* refactor: change `Handler.patch()` to include `fieldname` argument

This will enable `Handler`s with multiple fields to work for patching.
Necessary for `Time.patch()` and `Conference.patch()`

Also introduce `SingleFieldHandler.patch(cls,` ...`)` which dispatches to
`cls._patch()`.

* refactor: move gcal.GoogleCalendarInterface._isallday() to utils.is_all_day()

This enables its use by details.py.

* refactor: `details.Url` uses `details.URL_PROPS` to identify properties to get

The new `details.URL_PROPS` constant `OrderedDict` will also work for
use in `Url.patch()`, which should be implemented in the next commit.
This will allow automated handling of two property/fieldname pairs for
this `Handler`.

* refactor: add `exceptions.ReadonlyError`, `exceptions.ReadonlyCheckError`
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.

None yet

3 participants