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

Missing clasp support for Drive Activity API #513

Open
tehhowch opened this issue Jan 30, 2019 · 3 comments
Open

Missing clasp support for Drive Activity API #513

tehhowch opened this issue Jan 30, 2019 · 3 comments
Assignees

Comments

@tehhowch
Copy link

tehhowch commented Jan 30, 2019

Expected Behavior

Drive Activity API can be enabled via clasp apis enable driveactivity, and the project can continue to be worked with. This is expected because the API can be enabled via the web UI:
image
image

Actual Behavior

appsscript.json receives a null entry:
image

Any clasp operation using/modifying the manifest then fails (e.g. push, additional apis ____), since the null value cannot be consumed:
image
(Note that the enable/disable goes through to the GCP, but the local manifest file is not updated)

image

Steps to Reproduce the Problem

  1. clasp create --standalone
  2. clasp apis enable driveactivity
    image

Specifications

  • Node version (node -v): 8.11.2
  • Version (clasp -v): 2.0.1
  • OS (Mac/Linux/Windows): Win 10
@erickoledadevrel erickoledadevrel self-assigned this Jan 30, 2019
@erickoledadevrel
Copy link
Contributor

Thanks for the detailed issue report. There seem to be two problems here:

  1. The new advanced service DriveActivity hasn't been added to the PUBLIC_ADVANCED_SERVICES constant. Unfortunately there is no way to programmatically retrieve this information at the moment and it must be manually kept in sync with the Apps Script UI.
  2. When adding the advanced service to the manifest there is currently no error handling for when the enabled API isn't in the PUBLIC_ADVANCED_SERVICES enum. As seen here, if not found the null is added to the manifest.

Fixing the first problem should be a simple PR to add the details to the constant. Fixing the second problem involves checking for null, and if so skipping the manifest modification and logging a warning.

@grant - Does that sound reasonable?

@erickoledadevrel
Copy link
Contributor

Ping @grant.

@grant
Copy link
Contributor

grant commented Feb 15, 2019

Sounds fine @erickoledadevrel.
This is one of the features that tries to help the Apps Script developer experience, but is really an issue with the lack of a feature with the Apps Script API.

We should simply fail gracefully at least.

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

No branches or pull requests

3 participants