Add ability to import Google docs into posts #8756

Will-Howard · 2024-02-12T18:19:00Z

Based on these designs. There are a few differences from the designs, some of which are based on this slack thread, and some of which are things I changed (@agnesstenlund I can change them back if you want).

The changes from the thread are:

Removing the option to link your account

The additional changes I made:

I changed the copy a bit, including removing the "Import shared Google doc" header (because this is implied now there is only one option)
I made it so all the bits of the UI (the button and the "This will overwrite" message) show before you enter a url. I did this because I personally find it a bit annoying when you can't tell what all the steps are before you fill in part of a form (mentioned in the slack thread), and also now there is only one form I think it looks ok to have more info in one go

Summary of the logic

Clicking "Import Google doc" does the following:

Exports the doc as html, using a "service account" (that an admin has to log into to set up). For us the account is eaforum.posts@gmail.com. I decided to go with a plain gmail account to avoid any risk of exposing CEA files (because you'll be able to download any doc this account has access to), also it's a bit shorter
Do some conversion (see convertImportedGoogleDoc) of the html to make it work with ckeditor, this is mostly to replicate things we do on paste in ckeditor
Create a new revision and/or post depending on whether this is the new post or edit post form:
- For new posts, create a draft post with the converted html (which also creates a revision in a callback)
- For editing posts, create a new revision (this being the latest revision will cause it to load in the editor), but don't set this as the live revision on the post. This means they will need to hit publish to update the live post
Refresh the page

Screenshots

Things left to do, that I will aim to do before merging:

Add analytics
Add a reminder to admins (in the frontpage sidebar) to re-login to the service account when the session gets close to expiring
Stretch: Make the doc url pre-fill if you are on a post where you have previously imported from a google doc
Stretch: Add tests for the convertImportedGoogleDoc function (which replicates the logic in ckeditor)

┆Issue is synchronized with this Asana task by Unito

…rt-2024-02

agnesstenlund · 2024-02-20T12:34:12Z

A question–will it not also overwrite saved changes?

agnesstenlund · 2024-02-20T12:57:21Z

it takes 1 sec before the button become non-disabled, any chance that could be faster? It's long enough for me to get confused that it's not working:

Screen.Recording.2024-02-20.at.13.56.05.mov

agnesstenlund · 2024-02-20T12:58:18Z

fyi I changed my mind slightly about the colors/ UI when seeing it live–changed to black and grey:

agnesstenlund · 2024-02-20T13:00:59Z

If I delete the text in the input the button doesn't go back to disabled unless I refresh:

Screen.Recording.2024-02-20.at.14.00.20.mov

…/ForumMagnum into wh-google-doc-import-2024-02

Will-Howard · 2024-02-20T13:21:45Z

@agnesstenlund

A question–will it not also overwrite saved changes?

This is a bit of a semantic issue that is hard to get across in a short sentence, basically:

For changes where you've previously clicked "save as draft" or "publish", you will be able to restore them from version history, but it will "overwrite them" as in replacing what is currently in the editor
For changes you haven't saved, it will overwrite them completely and there is no way to get them back (except possibly via the "Do you want to restore your text" bar, but I'm not sure if this will work reliably

So I think the current wording is correct if you take overwrite to mean "delete in a way you can't recover easily"

it takes 1 sec before the button become non-disabled, any chance that could be faster? It's long enough for me to get confused that it's not working

It's not really possible to make this faster unfortunately without changing how it works. I expect it to be a bit faster on production though (maybe twice as fast, part of the time is looking things up in our database which will be faster, the rest is looking up the doc with google which will be the same).

If we did change how it works we could do something like un-disabling the button as soon as it gets a valid url (whether or not we have access to it), but then failing when you click the button if we don't have access (and still showing the message)

If I delete the text in the input the button doesn't go back to disabled unless I refresh:

Fixed this 👍

Will-Howard · 2024-02-20T14:59:08Z

Comments from Lizka:

Copy changes:

[Not exactly related to gdoc import] “Share draft” should IMO be “Share this draft” — next to the gdoc import it looks like it has something to do with gdocs…

Seems good 👍 , I've updated this

“Paste a link…” maybe is clearer if we write “Paste a link to your Google Doc here; make sure the anyone with the link can view or the Doc is shared with [email address].” (It’s more words, but less disorienting imo)

Hmm I prefer the original (bc it's fewer words), interested in what @agnesstenlund thinks. I think I'm assuming people will basically have a good understanding of how google doc permissions work so will get the idea without much explanation. Here's a side by side of the two wordings:

Love the red “We don’t have access” thing — allays basically all of my concerns

If I go and update the doc and don’t change anything on this tab, will it automatically update? Otherwise I’d add a “Re-upload” button or something if possible

Good catch, I've changed it so it doesn't update automatically. Unfortunately this means we now might be at risk of hitting a rate limit, although according to my calculation it should be able to tolerate ~80 users using it concurrently, which should hopefully be way more than we need

I don’t get the “Google Doc import service account” screenshot, but I assume it’s not meant to be viewable to users? If it is, I’d change that page.

Yup this is for admins (and only devs basically), users won't see this

agnesstenlund · 2024-02-20T15:26:37Z

Agree that the rewrite is too long. Since we say Google doc above I don't think it's necessary + the length of that line makes me slightly more confused about it?

agnesstenlund · 2024-02-20T15:27:37Z

could do something like "Paste your Google doc link here. It needs to be public or shared with eaforum.posts@gmail.com" if we think we should mention google doc

Will-Howard · 2024-02-20T17:33:52Z

could do something like "Paste your Google doc link here. It needs to be public or shared with eaforum.posts@gmail.com" if we think we should mention google doc

Eh 🤷‍♂️, I think it's hopefully obvious from the placeholder and button text

Will-Howard · 2024-02-20T18:01:00Z

packages/lesswrong/server/authenticationMiddlewares.ts

+  const googleClientId = googleClientIdSetting.get();
+  const googleOAuthSecret = googleOAuthSecretSetting.get();


I've set up a dev and prod client in the google cloud console (see here)

I think I know what you did here, but @Will-Howard, can you elaborate on the kind of credentials you set up?

The part I need to figure out is the consent screen, I think. How did that work for you?

Will-Howard · 2024-02-21T11:54:30Z

@oetherington btw the error in the console is a (non-serious) bug in the google api library, for which there is a fix that will be merged soon

jpaddison3 · 2024-02-21T14:58:14Z

I love that when you link to that fix it puts a backlink in the referenced issue. Like, "we're waiting on you bro".

oetherington

LGTM.

I left quite a few comments but I don't think anything is necessarily blocking, except maybe admin gating the auth routes (if I'm understanding correctly how those are supposed to work).

oetherington · 2024-02-21T22:14:57Z

packages/lesswrong/server/posts/googleDocImport.ts

+const clientIdSetting = new DatabaseServerSetting<string | null>('googleDocImport.oAuth.clientId', null)
+const clientSecretSetting = new DatabaseServerSetting<string | null>('googleDocImport.oAuth.secret', null)
+
+let _oAuthClient: OAuth2Client | null = null;


I don't think a preceding underscore is the correct naming convention for globals. Generally you'd use OAUTH_CLIENT or g_oAuthClient. A preceding underscore has the special meaning of implying that the variable is unused, and making linters ignore it if that's the case.

oetherington · 2024-02-21T22:16:22Z

packages/lesswrong/server/authenticationMiddlewares.ts

@@ -398,7 +478,7 @@ export const addAuthMiddlewares = (addConnectHandler: AddMiddlewareType) => {
    });
  }

-  addConnectHandler('/auth/google/callback', (req: Request, res: Response, next: NextFunction) => {
+  addConnectHandler('/auth/google/callback', (req: AnyBecauseTodo, res: AnyBecauseTodo, next: AnyBecauseTodo) => {


I think this line is an incorrect merge conflict resolution

oetherington · 2024-02-21T22:20:59Z

packages/lesswrong/components/sunshineDashboard/SunshineGoogleServiceAccount.tsx

+  return (
+    <div className={classes.root}>
+      <Link to={"/admin/googleServiceAccount"}>
+      {message}


nit: indentation

oetherington · 2024-02-21T22:24:38Z

packages/lesswrong/components/admin/AdminHome.tsx

@@ -74,6 +74,7 @@ const AdminHome = ({ classes }: {
        <li><Link className={classes.link} to="/admin/synonyms">Search Synonyms</Link></li>
        <li><Link className={classes.link} to="/admin/tagMerge">{taggingNameCapitalSetting.get()} Merging Tool</Link></li>
        <li><span className={classes.link} onClick={refreshDbSettings}>Refresh DB Settings</span></li>
+        <li><Link className={classes.link} to="/admin/googleServiceAccount">Google Doc import service account</Link></li>


I think the <Loading /> on the line below corresponse to the "Refresh DB Settings" option above, so this probably shouldn't go in between them (it's a bit janky but, hey, it's an admin page).

oetherington · 2024-02-26T11:47:05Z

package.json

@@ -209,6 +209,7 @@
    "express-session": "^1.17.1",
    "feedparser-promised": "^1.2.2",
    "fp-ts": "^2.13.1",
+    "googleapis": "^133.0.0",


WRT the error in the package.json for this library, it might be worth making a manual patch (which should be very straight forward). The issue has existed for 6 weeks and the PR to fix it has already been open for 3 weeks, and I have less than 0 faith in Google being competant enough to actually fix it in a timely manner.

I'll give them a couple more weeks, but I'll set a reminder with a deadline so we don't just get stuck with this forever

oetherington · 2024-02-26T13:39:22Z

packages/lesswrong/server/authenticationMiddlewares.ts

@@ -20,6 +20,9 @@ import { Request, Response, NextFunction, json } from "express";
 import { AUTH0_SCOPE, loginAuth0User, signupAuth0User } from './authentication/auth0';
 import { IdFromProfile, UserDataFromProfile, getOrCreateForumUser } from './authentication/getOrCreateForumUser';
 import { promisify } from 'util';
+import { OAuth2Client } from 'google-auth-library';


Given that this file already has multiple oauth clients, maybe this should be OAuth2Client as GoogleOAuth2Client

oetherington · 2024-02-26T13:41:17Z

packages/lesswrong/server/authenticationMiddlewares.ts

+  const callbackUrl = "google_oauth2callback"
+  const oauth2Client = new OAuth2Client(googleClientId, googleOAuthSecret, `${getSiteUrl()}${callbackUrl}`);
+
+  addConnectHandler('/auth/linkgdrive', (req: Request, res: Response) => {


Should these two handlers be admin gated?

Yes they should! Great catch

oetherington · 2024-02-26T13:43:10Z

packages/lesswrong/server/editor/conversionUtils.ts

+/**
+ * Convert the footnotes we get in google doc html to a format ckeditor can understand. This is mirroring the logic
+ * in the footnotes plugin (see e.g. public/lesswrong-editor/src/ckeditor5-footnote/src/footnoteEditing/googleDocsFootnotesNormalizer.js)
+ */


Can we put a comment in that JS file as well so if it gets updated those updates are more likely to be copied here too?

oetherington · 2024-02-26T13:56:28Z

packages/lesswrong/server/resolvers/postResolvers.ts

+        return null;
+      }
+
+      return await canAccessGoogleDoc(fileUrl)


nit: superfluous async/await

oetherington · 2024-02-26T14:01:34Z

packages/lesswrong/server/editor/conversionUtils.ts

+  const { html: withRehostedImages } = await convertImagesInHTML(html, postId, url => url.includes("googleusercontent"))
+  const withConvertedFootnotes = googleDocConvertFootnotes(withRehostedImages)
+  const withNormalisedRedirects = googleDocRemoveRedirects(withConvertedFootnotes)
+  const ckEditorMarkup = await dataToCkEditor(withNormalisedRedirects, "html")


A couple of import issues I noticed that probably need to be handled here:

The document title got imported as normal text at the top of the document, but maybe we should use it to set the post title and remove it from the document body?

Bold, italics and underline don't work

Internal links still point to the google doc, maybe we can fix this now or is it still difficult?

The document title got imported as normal text at the top of the document, but maybe we should use it to set the post title and remove it from the document body?

When creating a new post it already sets the actual title of the doc as the title, I guess it will often be repeated in the main body but it doesn't seem that costly for people to remove it manually

Bold, italics and underline don't work

Thanks, will fix. If it's not completely straightforward I might do it as a followup (but still today)

Internal links still point to the google doc, maybe we can fix this now or is it still difficult?

We can fix that now, I will do this one as a followup, probably also today

Ok I did the bold/italics one, and made a ticket for internal links

Will-Howard added 5 commits February 12, 2024 18:30

google doc import for editing posts broadly working end to end

bcb21ea

working on both new and existing posts

e62609c

unlinking accounts working

a4e77f2

update to do list

d95d809

progress on version restoration

1882813

Will-Howard force-pushed the wh-google-doc-import-2024-02 branch from a5f6445 to 1882813 Compare February 13, 2024 12:13

Will-Howard added 3 commits February 13, 2024 15:22

version restoration done

a071833

fix reloading of page after import

f0ded16

remove comment

67cb296

Will-Howard mentioned this pull request Feb 13, 2024

Improve post version history editor (required for Google doc import) #8764

Merged

Will-Howard added 6 commits February 14, 2024 15:13

temp commit, some progress on paste issues

96f2a3a

footnote importing largely working

2905df1

remove google redirects from imported html

aa8fb2e

rehost images on import

f208fee

some progress on google doc UI

9775025

fix dependency cycle

f7b34f9

Will-Howard mentioned this pull request Feb 19, 2024

Refactor to allow arbitrary form group layout components #8787

Merged

Will-Howard added 9 commits February 19, 2024 08:53

Merge branch 'wh-form-group-refactor-2024-02' into wh-google-doc-impo…

7244365

…rt-2024-02

port post editor top bar to FormGroupPostTopBar

c0cfbf0

progress on final UI

4dcc518

UI largely done

4d8229e

some special breakpoint logic

e938834

some cleanup

f480abd

various cleanup items

e71ddc5

fix errors

b140ed1

merge with master

927b87a

Will-Howard changed the base branch from master to wh-version-history-2024-02 February 19, 2024 15:54

Will-Howard changed the base branch from wh-version-history-2024-02 to master February 19, 2024 15:54

upgrade googleapis

392ed1d

Will-Howard changed the base branch from master to wh-form-group-refactor-2024-02 February 19, 2024 16:09

UI changes to doc import button

e6520f8

Will-Howard added 2 commits February 20, 2024 13:07

add warning in sunshine sidebar

b328c23

Merge branch 'wh-google-doc-import-2024-02' of github.com:ForumMagnum…

5a046d1

…/ForumMagnum into wh-google-doc-import-2024-02

fix clearing url not disabling button

4aa5c6d

Will-Howard assigned oetherington Feb 20, 2024

Will-Howard added 2 commits February 20, 2024 14:53

re-check access

7a0d843

'Share draft' -> 'Share this draft'

d18c53e

Will-Howard commented Feb 20, 2024

View reviewed changes

oetherington approved these changes Feb 26, 2024

View reviewed changes

oetherington assigned Will-Howard and unassigned oetherington Feb 27, 2024

Will-Howard changed the base branch from wh-form-group-refactor-2024-02 to master February 29, 2024 12:46

Will-Howard added 3 commits February 29, 2024 13:37

address straightforward comments

bb019c8

make hasGoogleDocImport a setting, handle bold and italics

4b7df71

merge with master

dc82f6a

Will-Howard merged commit 7fa6a30 into master Feb 29, 2024
12 checks passed

This was referenced Feb 29, 2024

Use right credentials for signing in to google doc service account #8841

Merged

Fix deployment issue with google doc import #8847

Merged

Fix buttons by making allowed classes more permissive #8852

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to import Google docs into posts #8756

Add ability to import Google docs into posts #8756

Will-Howard commented Feb 12, 2024 •

edited

agnesstenlund commented Feb 20, 2024 •

edited

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

Will-Howard commented Feb 20, 2024

Will-Howard commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024 •

edited

Will-Howard commented Feb 20, 2024

Will-Howard Feb 20, 2024

darkruby501 Mar 6, 2024

Will-Howard commented Feb 21, 2024

jpaddison3 commented Feb 21, 2024

oetherington left a comment

oetherington Feb 21, 2024

oetherington Feb 21, 2024

oetherington Feb 21, 2024

oetherington Feb 21, 2024

oetherington Feb 26, 2024

Will-Howard Feb 29, 2024

oetherington Feb 26, 2024

oetherington Feb 26, 2024

Will-Howard Feb 29, 2024

oetherington Feb 26, 2024

oetherington Feb 26, 2024

oetherington Feb 26, 2024

Will-Howard Feb 29, 2024

Will-Howard Feb 29, 2024

		const googleClientId = googleClientIdSetting.get();
		const googleOAuthSecret = googleOAuthSecretSetting.get();

Add ability to import Google docs into posts #8756

Add ability to import Google docs into posts #8756

Conversation

Will-Howard commented Feb 12, 2024 • edited

Summary of the logic

Screenshots

agnesstenlund commented Feb 20, 2024 • edited

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

Will-Howard commented Feb 20, 2024

Will-Howard commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024

agnesstenlund commented Feb 20, 2024 • edited

Will-Howard commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Will-Howard commented Feb 21, 2024

jpaddison3 commented Feb 21, 2024

oetherington left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Will-Howard commented Feb 12, 2024 •

edited

agnesstenlund commented Feb 20, 2024 •

edited

agnesstenlund commented Feb 20, 2024 •

edited