Post preview with blobs and mentions #462
Conversation
and cleanup blob addition to text
also make sure to only display content warning in summary for subtopics
replaces immediatly if there is one match, but just spews multiple ones. * add silly in-memory about:name cache rational is also commented the code. the tl:dr; is oasis already doesnt use ssb-about or other indexing plugins. butt for @mentions we need a quick way to lookup names to feeds, otherwise we block the preview flow with long query times.
lowercase the searches and see if they match the beginning of a name.
this time without cryptix's basic blob test (to see if CI completes)
after having restored the package-lock.json to that of the master branch, this PR now uses [email protected], which has common-good check as a command. i ran it manually as $(npm bin)/common-good check, and fixed whatever it complained about.
|
alright so i have pretty much rectified everything i can think of to make the CI pass. if anyone else has ideas on what the issue is (the timeout?) do reach out :)) as regards the code, other than the package-lock fix, i think everything was put through its paces & stable at the onset of the PR. i fear that the changes i introduced just to please the CI / common-good might have caused issues. |
|
i believe i'm able to reproduce the test failure locally. it seems that 22/22 asserts are passing, but the suite never completes and tap never exits. i added a i've modified i am also able to reproduce the hang by running i did notice that the tests are connecting to my real ssb-server (should this be mocked?), and if i shut the server down while the tests are hung at the end, the test throws an exception and exits (with code 7): maybe, despite if i do not have an ssb-server running when i execute the tests, I do see the following console output at the end of asserts: |
|
Not sure if this is the right place to document this issue, but I was testing blob uploads just now, and kept running into an error whenever I tried to preview.
the error shown is |
|
i added a i am not a javascript dev and relatively unfamiliar with the node ecosystem, but hopefully this sets ya'll in the right direction. |
|
@aadilayub which commit are you running off of? :) |
|
The re-indexing issue is now fixed as of commit 4d5c571. Tested by running Oasis and Patchwork 3.18.0 one at a time, back and forth. |
|
@cblgh I'm running off 4d5c571df3380f7544c00af9b5411c7dfba555c2 |
|
(tiny update: just taking a little break before i begin bashing my head against ci issues again, i hate dealing with ci :) |
|
I know next to nothing about travis, and had to look up what tap is, but other people also had problems when running tap tests against koa code, with the tests not terminating: |
|
@aadilayub I just ran into the error you described (#462 (comment)). I believe it's some kind of race and looking at the code again, I think i have a better way of doing that. There was no reason to have the |
fixes TypeError: Cannot read property 'startsWith' of undefined
|
i'm back at it again! the plan is to spend today reconnoitering around the spooky timeouts which cause Travis to fall on his face |
cblgh
force-pushed
the
post-preview-pr-ready
branch
from
6ca5812 to
e7ea4db
Compare
|
👀 now to see if it works without uncommenting a particularly troublesome route, all while leaving the workaround to definitively stop the server intact in the function call that should definitively stop the server |
|
Nice! 🎉 I spent a bit of time on this yesterday too -- I thought it was the Which route is troublesome? I've put in lots of work to avoid |
|
nvm, looks like you nailed the problem down to the summaries route? that's very surprising, that's been around for a while and hasn't been on my radar as a trouble-maker |
|
Good news: I found the problem! Bad news: The only fix I've tried is "delete the entire chunk of code". Here's a diff against e567443: click to expanddiff --git a/src/index.js b/src/index.js
index 18e02ee..012f5b4 100755
--- a/src/index.js
+++ b/src/index.js
@@ -1059,9 +1059,6 @@ const app = http({ host, port, middleware, allowHost });
// stream closing" errors everywhere and breaks the tests. :/
app._close = () => {
cooler.close();
- setTimeout(() => {
- process.exit();
- }, 20000);
};
module.exports = app;
diff --git a/src/models.js b/src/models.js
index 3e91d19..08eb994 100644
--- a/src/models.js
+++ b/src/models.js
@@ -195,55 +195,6 @@ module.exports = ({ cooler, isPublic }) => {
});
};
- cooler.open().then((ssb) => {
- console.time("about-name-warmup"); // benchmark the time it takes to stream all existing about messages
- pull(
- ssb.query.read({
- live: true, // keep streaming new messages as they arrive
- query: [
- {
- $filter: {
- // all messages of type:about that have a name field that is typeof string
- value: {
- content: {
- type: "about",
- name: { $is: "string" },
- },
- },
- },
- },
- ],
- }),
- pull.filter((msg) => {
- // backlog of data is done, only new values from now on
- if (msg.sync && msg.sync === true) {
- console.timeEnd("about-name-warmup");
- transposeLookupTable(); // fire once now
- setInterval(transposeLookupTable, 1000 * 60); // and then every 60 seconds
- return false;
- }
- // only pick messages about self
- return msg.value.author == msg.value.content.about;
- }),
- pull.drain((msg) => {
- const name = msg.value.content.name;
- const ts = msg.value.timestamp;
- const feed = msg.value.author;
-
- const newEntry = { name, ts };
- const currentEntry = feeds_to_name[feed];
- if (typeof currentEntry == "undefined") {
- dirty = true;
- feeds_to_name[feed] = newEntry;
- } else if (currentEntry.ts < ts) {
- // overwrite entry if it's newer
- dirty = true;
- feeds_to_name[feed] = newEntry;
- }
- })
- );
- });
-
models.about = {
publicWebHosting: async (feedId) => {
const result = await getAbout({
diff --git a/test/basic.js b/test/basic.js
index 2f0fca5..ef50631 100644
--- a/test/basic.js
+++ b/test/basic.js
@@ -15,7 +15,7 @@ const paths = [
"/profile/edit",
"/public/latest",
"/public/latest/extended",
- // "/public/latest/summaries",
+ "/public/latest/summaries",
"/public/latest/threads",
"/public/latest/topics",
"/public/popular/day",I've tried only deleting the |
Problem: Recently there was a PR [0] merged with a quickfix to avoid some test failures, which is something I've been trying to avoid. While `process.exit()` works fine, I'm worried that it means we don't understand what's happening under the hood, *plus* I have the [maybe unjustified?] worry that it might kill the process during a database write or something dangerous. It looks like this particular test hang was caused by both a stream and some number of intervals that were left open. Solution: Provide a way to close the stream and intervals in `index.js` and ensure that we do that before closing the server. [0]: fraction#462
|
Thanks for this patch, I'm super excited to have it in! |
|
yaaaaaaay!!!!!!!!!!!!!!!!!!!!!! |
This PR contains the outcome from
%9NXKauuXCgJCHUaIXFGbpnXucAmvZ0QyQFJOAzURspc=.sha256: @cblgh & @cryptix's sprint to add post previews and blob uploads to the post composer. Next to those two big features @cblgh also implemented a variety of visual tweaks, including a more concise thread rendering, and we found enough time to improve the@mentionsuggestion feature.As such we think it should:
Screenshots
Post preview
This is the preview flow we implemented. If you mention someone, and the match is ambiguous we show all the probable matches as cards, with a prepared markdown link that can be copied into the text. This is less complex and than an initial idea we had (which made use of
<input type:radio>for each name).Thread view
Posts now stick together much closer. We think it makes it clearer what is a conversation with replies compared to the latest or popular view where it's just a list of (unrelated) posts.
Miscellaneous
re oasis-desktop:
Apart from one weird bug in the database server, it all went quite swell.. but that one is a bit frightning as cryptix couldn't figure out where it's comming from. The TL;dr for that one is: it randomly stopped showing names and avatar images for (some) people..
Our current bit-sized summary action plan is: make a release with just the new features (as non of that requires updating the database) and make a -beta release with most recent dependencies to test this further.
A Blobby Caveat
A caveat with the current blob upload is that, after a file has been attached, the preview needs to be rendered (by clicking the Preview button) before anything is visible in the composer. This can be slightly confusing. The alternative would be to rely on the browser native file input, which is notoriously hard to style.
other nice things
and).startsWith()so you can just type the beginning if your not 100% sure how it's spelled