Merge pull request #9854 from cdrini/feature/merge-ui-configs

Make MergeUI/all vue components easier to test locally

.gitignore

@@ -1,3 +1,4 @@
+openlibrary/components/_dev.js
 *venv
 *.pyc
 *~

openlibrary/components/LibraryExplorer/components/BookRoom.vue

@@ -81,7 +81,7 @@ import ExpandIcon from './icons/ExpandIcon.vue';
 import debounce from 'lodash/debounce';
 import Vue from 'vue';
 import { decrementStringSolr, hierarchyFind, testLuceneSyntax } from '../utils.js';
-import CONFIGS from '../configs';
+import CONFIGS from '../../configs';
 /** @typedef {import('../utils.js').ClassificationNode} ClassificationNode */
 
 /**

openlibrary/components/LibraryExplorer/components/BooksCarousel.vue

@@ -22,7 +22,7 @@
 
 <script>
 import FlatBookCover from './FlatBookCover';
-import CONFIGS from '../configs';
+import CONFIGS from '../../configs';
 
 export default {
     components: { FlatBookCover },

openlibrary/components/LibraryExplorer/components/FlatBookCover.vue

@@ -17,7 +17,7 @@
 
 
 <script>
-import CONFIGS from '../configs';
+import CONFIGS from '../../configs';
 import { hashCode } from '../utils.js';
 
 export default {

openlibrary/components/LibraryExplorer/components/LibraryToolbar.vue

@@ -205,7 +205,7 @@ import SettingsIcon from './icons/SettingsIcon';
 import FilterIcon from './icons/FilterIcon';
 import SortIcon from './icons/SortIcon';
 import FeedbackIcon from './icons/FeedbackIcon';
-import CONFIGS from '../configs';
+import CONFIGS from '../../configs';
 import Multiselect from 'vue-multiselect';
 
 export default {

openlibrary/components/LibraryExplorer/components/OLCarousel.vue

@@ -38,7 +38,7 @@
 import BooksCarousel from './BooksCarousel.vue';
 import debounce from 'lodash/debounce';
 import Vue from 'vue';
-import CONFIGS from '../configs';
+import CONFIGS from '../../configs';
 // import * as Vibrant from "node-vibrant";
 
 // window.Vibrant = Vibrant;

openlibrary/components/MergeUI.vue

@@ -1,23 +1,28 @@
 <template>
   <div id="app">
-    <MergeTable :olids="olids" :show_diffs="show_diffs" :primary="primary" ref="mergeTable"/>
-    <div class="action-bar">
-        <div class="comment-input">
-            <label for="comment">Comment: </label>
-            <input name="comment" v-model="comment" type="text">
-        </div>
-        <div class="btn-group">
-            <button class="merge-btn" @click="doMerge" :disabled="isDisabled">{{mergeStatus}}</button>
-            <button class="reject-btn" v-if="showRejectButton" @click="rejectMerge">Reject Merge</button>
-        </div>
-        <div id="diffs-toggle">
-            <label>
-                <input type="checkbox" title="Show textual differences" v-model="show_diffs" />
-                Show text diffs
-            </label>
-        </div>
+    <div v-if="!olids.length">
+        No records to merge. Specify some records in the url like so: <code>?records=OL123W,OL234W</code>
     </div>
-    <pre v-if="mergeOutput">{{mergeOutput}}</pre>
+    <template v-else>
+        <MergeTable :olids="olids" :show_diffs="show_diffs" :primary="primary" ref="mergeTable"/>
+        <div class="action-bar">
+            <div class="comment-input">
+                <label for="comment">Comment: </label>
+                <input name="comment" v-model="comment" type="text">
+            </div>
+            <div class="btn-group">
+                <button class="merge-btn" @click="doMerge" :disabled="isDisabled">{{mergeStatus}}</button>
+                <button class="reject-btn" v-if="showRejectButton" @click="rejectMerge">Reject Merge</button>
+            </div>
+            <div id="diffs-toggle">
+                <label>
+                    <input type="checkbox" title="Show textual differences" v-model="show_diffs" />
+                    Show text diffs
+                </label>
+            </div>
+        </div>
+        <pre v-if="mergeOutput">{{mergeOutput}}</pre>
+    </template>
   </div>
 </template>
 
@@ -47,7 +52,8 @@ export default {
         },
         canmerge: {
             type: String,
-            required: true
+            required: false,
+            default: 'true',
         }
     },
     data() {
@@ -61,7 +67,12 @@ export default {
     },
     computed: {
         olids() {
-            return this.url.searchParams.get('records', '').split(',')
+            const olidsString = this.url.searchParams.get('records');
+            if (!olidsString) return [];
+            return olidsString
+                .split(',')
+                // Ignore trailing commas
+                .filter(Boolean);
         },
 
         isSuperLibrarian() {

openlibrary/components/MergeUI/MergeTable.vue

@@ -48,6 +48,7 @@ import Vue from 'vue';
 import AsyncComputed from 'vue-async-computed';
 import MergeRow from './MergeRow.vue';
 import { merge, get_editions, get_lists, get_bookshelves, get_ratings } from './utils.js';
+import CONFIGS from '../configs.js';
 
 Vue.use(AsyncComputed);
 
@@ -60,10 +61,15 @@ function fetchRecord(olid) {
         M: 'books',
         A: 'authors'
     }[olid[olid.length - 1]];
+    let base = '';
+    if (CONFIGS.OL_BASE_BOOKS) {
+        base = CONFIGS.OL_BASE_BOOKS;
+    } else {
+        // FIXME Fetch from prod openlibrary.org, otherwise it's outdated
+        base = location.host.endsWith('.openlibrary.org') ? 'https://openlibrary.org' : '';
+    }
     const record_key = `/${type}/${olid}`;
-    // FIXME Fetch from prod openlibrary.org, otherwise it's outdated
-    const url = location.host.endsWith('.openlibrary.org') ? `https://openlibrary.org${record_key}.json` : `${record_key}.json`;
-    return fetch(url).then(r => {
+    return fetch(`${base}${record_key}.json`).then(r => {
         return (r.ok) ? r.json() : {key: record_key, type: {key: '/type/none'}, error: r.statusText};
     });
 }

openlibrary/components/MergeUI/utils.js

@@ -1,6 +1,7 @@
 /* eslint no-console: 0 */
 import _ from 'lodash';
 import { approveRequest, declineRequest, createRequest, REQUEST_TYPES } from '../../plugins/openlibrary/js/merge-request-table/MergeRequestService'
+import CONFIGS from '../configs.js';
 
 const collator = new Intl.Collator('en-US', {numeric: true})
 export const DEFAULT_EDITION_LIMIT = 200
@@ -125,30 +126,35 @@ export function make_redirect(master_key, dupe) {
 
 export function get_editions(work_key) {
     const endpoint = `${work_key}/editions.json`;
-    // FIXME Fetch from prod openlibrary.org, otherwise it's outdated
-    const url = location.host.endsWith('.openlibrary.org') ? `https://openlibrary.org${endpoint}` : endpoint;
-    return fetch(`${url}?${new URLSearchParams({limit: DEFAULT_EDITION_LIMIT})}`).then(r => {
+    let base = '';
+    if (CONFIGS.OL_BASE_BOOKS) {
+        base = CONFIGS.OL_BASE_BOOKS;
+    } else {
+        // FIXME Fetch from prod openlibrary.org, otherwise it's outdated
+        base = location.host.endsWith('.openlibrary.org') ? 'https://openlibrary.org' : '';
+    }
+    return fetch(`${base}${endpoint}?${new URLSearchParams({limit: DEFAULT_EDITION_LIMIT})}`).then(r => {
         if (r.ok) return r.json();
         if (confirm(`Network error; failed to load editions for ${work_key}. Click OK to reload.`)) location.reload();
     });
 }
 
 export function get_lists(key, limit=10) {
-    return fetch(`${key}/lists.json?${new URLSearchParams({ limit })}`).then(r => {
+    return fetch(`${CONFIGS.OL_BASE_BOOKS}${key}/lists.json?${new URLSearchParams({ limit })}`).then(r => {
         if (r.ok) return r.json();
         return {error: true};
     });
 }
 
 export function get_bookshelves(key) {
-    return fetch(`${key}/bookshelves.json`).then(r => {
+    return fetch(`${CONFIGS.OL_BASE_BOOKS}${key}/bookshelves.json`).then(r => {
         if (r.ok) return r.json();
         return {error: true};
     });
 }
 
 export function get_ratings(key) {
-    return fetch(`${key}/ratings.json`).then(r => {
+    return fetch(`${CONFIGS.OL_BASE_BOOKS}${key}/ratings.json`).then(r => {
         if (r.ok) return r.json();
         return {error: true};
     });
@@ -216,7 +222,8 @@ function save_many(items, comment, action, data) {
         '42-action': action,
         '42-data': JSON.stringify(data),
     };
-    return fetch('/api/save_many', {
+
+    return fetch(`${CONFIGS.OL_BASE_SAVES}/api/save_many`, {
         method: 'POST',
         headers,
         body: JSON.stringify(items)

openlibrary/components/README.md

@@ -8,13 +8,28 @@ The building of these files happens on `make components`.
 
 ## Live-reloading dev server
 
-First, update `openlibrary/components/dev.js` to use the component you're developing instead of `HelloWorld.vue`
-Then, outside the docker environment, run:
+The Vue components follow the following structure:
+
+```
+openlibrary/components/{MainComponent}.vue  -- The entrypoint to the component
+openlibrary/components/{MainComponent}/...  -- Any sub components, utils, etc.
+```
+
+**Outside the docker environment**, run:
 
 ```shell script
-npx @vue/cli-service serve openlibrary/components/dev.js
+npm install --no-audit
+npm run serve --component=HelloWorld
 ```
 
+Changing `HelloWorld` to be the name of the main component you want to work on.
+
+For apps that are configured for it (like `LibraryExplorer` and `MergeUI`), when run
+in this mode, the vue server will make requests to production openlibrary.org
+for data like books, search results, covers, etc. You can configure where it fetches data
+from by setting url parameters on the running app, eg `?ol_base=http://localhost:8080`. See
+`openlibrary/components/configs.js` for all the available url parameters.
+
 ## Caveats
 
 - Currently does not support IE11 because it's using web components (See https://caniuse.com/custom-elementsv1 )

openlibrary/components/LibraryExplorer/configs.js → openlibrary/components/configs.js

@@ -1,3 +1,6 @@
+// Useful configs that make testing the app easier. Exposes url parameters
+// to the app that you can use to override eg where to fetch data from.
+
 const urlParams = new URLSearchParams(location.search);
 
 const IS_VUE_APP =  document.title === 'Vue App';
@@ -8,12 +11,16 @@ const CONFIGS = {
     OL_BASE_SEARCH: urlParams.get('ol_base_search') || OL_BASE_DEFAULT || '',
     OL_BASE_BOOKS: urlParams.get('ol_base_books') || OL_BASE_DEFAULT || '',
     OL_BASE_LANGS: urlParams.get('ol_base_langs') || OL_BASE_DEFAULT || '',
+    // Make the save location explicitly different from ol_base to avoid
+    // accidentally triggering saves to prod (which shouldn't work anyways
+    // due to cookies, but just in case!)
+    OL_BASE_SAVES: urlParams.get('ol_base_saves') || '',
     OL_BASE_PUBLIC: urlParams.get('ol_base') || 'openlibrary.org',
     DEBUG_MODE: urlParams.get('debug') === 'true',
     LANG: urlParams.get('lang'),
 };
 
-for (const key of ['OL_BASE_COVERS', 'OL_BASE_SEARCH', 'OL_BASE_BOOKS', 'OL_BASE_LANGS']) {
+for (const key of ['OL_BASE_COVERS', 'OL_BASE_SEARCH', 'OL_BASE_BOOKS', 'OL_BASE_LANGS', 'OL_BASE_SAVES']) {
     if (CONFIGS[key] && !CONFIGS[key].startsWith('http')) {
         CONFIGS[key] = `https://${CONFIGS[key]}`;
     }

openlibrary/components/serve-component.js

@@ -0,0 +1,18 @@
+/* eslint-env node */
+const fs = require('fs');
+
+const componentName = process.env.npm_config_component || 'HelloWorld';
+
+// Create a copy of openlibrary/components/dev.js to _dev.js
+// Replace "HelloWorld" in openlibrary/components/_dev.js with the value of componentName
+fs.readFile('openlibrary/components/dev.js', 'utf8', (err, data) => {
+    if (err) {
+        throw err;
+    }
+    const result = data.replace(/HelloWorld/g, componentName);
+    fs.writeFile('openlibrary/components/_dev.js', result, 'utf8', (err) => {
+        if (err) {
+            throw err;
+        }
+    });
+});

package.json

@@ -28,6 +28,7 @@
     "lint-fix": "npm run lint-fix:js && npm run lint-fix:css",
     "lint-fix:js": "eslint --fix --ext js,vue .",
     "lint-fix:css": "stylelint --fix ./**/*.{less,vue}",
+    "serve": "node openlibrary/components/serve-component.js && npx @vue/cli-service serve openlibrary/components/_dev.js",
     "test": "npm run test:js && bundlesize",
     "test:js": "jest",
     "storybook": "storybook dev -p 6006",

vue.config.js

@@ -2,4 +2,23 @@
 module.exports = {
     lintOnSave: false,
     publicPath: '/static/components/',
+
+    // Add support for running from gitpod
+    ...(process.env.GITPOD_WORKSPACE_ID ? {
+        devServer: {
+            allowedHosts: [
+                // It'll pick the first free one, so will be 8080 if OL not running, otherwise 8081.
+                // The rest are just in case/if you run multiple
+                `8080-${process.env.GITPOD_WORKSPACE_ID}.${process.env.GITPOD_WORKSPACE_CLUSTER_HOST}`,
+                `8081-${process.env.GITPOD_WORKSPACE_ID}.${process.env.GITPOD_WORKSPACE_CLUSTER_HOST}`,
+                `8082-${process.env.GITPOD_WORKSPACE_ID}.${process.env.GITPOD_WORKSPACE_CLUSTER_HOST}`,
+                `8083-${process.env.GITPOD_WORKSPACE_ID}.${process.env.GITPOD_WORKSPACE_CLUSTER_HOST}`,
+            ],
+            client: {
+                webSocketURL: {
+                    port: 443,
+                },
+            },
+        },
+    } : {}),
 };