Page MenuHomePhabricator

ashoat (Ashoat)
UserAdministrator

Projects

User does not belong to any projects.

User Details

User Since
Jul 20 2020, 9:28 AM (165 w, 5 d)
Roles
Administrator

Recent Activity

Today

ashoat added a comment to D9265: [lib/web/native] Add IntegrityStore.

Do you mean just blocking until all hashes are complete (like it is currently) or something smarter (e.g. we send a request to the server to delay the state check)?

Sat, Sep 23, 12:44 PM

Yesterday

ashoat added a comment to D9265: [lib/web/native] Add IntegrityStore.

iOS emulator takes about 1ms per one thread and the speed should be O(n). In the case from the linear task where the state check took 53s, there was about 2000 threads, so logging in would take additional 2s

Fri, Sep 22, 5:48 AM

Thu, Sep 21

ashoat accepted D9262: [lib] Add `ThreadActivityStore` to Redux.
In D9262#272510, @atul wrote:

In the past, we would need to update websites-responders.js when adding a new field to Redux state. Now that Redux state for the web is fetched via an API call, is there a different place we should be updating?

Based on my reading of https://phab.comm.dev/D9145, the "new place" is web/redux/default-state.js which I did update in this diff.

Thu, Sep 21, 7:01 PM
ashoat requested changes to D9262: [lib] Add `ThreadActivityStore` to Redux.

In the past, we would need to update websites-responders.js when adding a new field to Redux state. Now that Redux state for the web is fetched via an API call, is there a different place we should be updating?

Thu, Sep 21, 2:14 PM
ashoat accepted D9223: [nix] add logs to console while waiting for MariaDB to come up.
Thu, Sep 21, 1:01 PM
ashoat accepted D9235: [lib] introduce createPendingPersonalThread function.
Thu, Sep 21, 1:00 PM
ashoat closed D9251: [keyserver] Remove unused exports in message-fetchers.js.
Thu, Sep 21, 9:02 AM
ashoat committed rCOMMa301a42f0654: [keyserver] Remove unused exports in message-fetchers.js (authored by ashoat).
[keyserver] Remove unused exports in message-fetchers.js
Thu, Sep 21, 9:01 AM
ashoat added a comment to D9255: [native] Use threadInfo in setClientDBStoreActionType.

Thanks @michal, that all makes sense.

Thu, Sep 21, 8:03 AM
ashoat added a reviewer for D9255: [native] Use threadInfo in setClientDBStoreActionType: atul.

I’d like to make sure @atul takes a look from the performance angle. Including this in ThreadStore is risky... any changes to the hashes will force every selector that depends on ThreadStore to re-run. If everything is memoized correctly, the performance cost should be limited. But if not there's a risk of hurting client performance by forcing unnecessary renders, especially if the actions "trickle in" separate from other changes to ThreadStore. (I assume "trickling in" would at least be necessary for a fresh log-in, where calculating all the hashes right away would be prohibitively expensive.)

Thu, Sep 21, 6:42 AM

Wed, Sep 20

ashoat closed D9250: [keyserver] Increase timeout for getInitialReduxState.
Wed, Sep 20, 2:32 PM
ashoat committed rCOMM9595f5ed54d1: [keyserver] Increase timeout for getInitialReduxState (authored by ashoat).
[keyserver] Increase timeout for getInitialReduxState
Wed, Sep 20, 2:32 PM
ashoat requested changes to D9223: [nix] add logs to console while waiting for MariaDB to come up.

What is the point of the outer if here...?

Wed, Sep 20, 2:30 PM
ashoat requested changes to D9235: [lib] introduce createPendingPersonalThread function.
Wed, Sep 20, 2:28 PM
ashoat requested review of D9251: [keyserver] Remove unused exports in message-fetchers.js.

Android failure is unrelated

Wed, Sep 20, 1:31 PM
ashoat accepted D9204: [lib] introduce threadInfosSelectorForThreadType selector.

Thanks for explaining!

Wed, Sep 20, 1:30 PM
ashoat requested review of D9250: [keyserver] Increase timeout for getInitialReduxState.
Wed, Sep 20, 1:19 PM
ashoat requested changes to D9249: [lib] Extend RobotextParams to include parentThreadInfo.
Wed, Sep 20, 11:55 AM
ashoat added inline comments to D9249: [lib] Extend RobotextParams to include parentThreadInfo.
Wed, Sep 20, 11:52 AM
ashoat committed rCOMM67aea23e378a: [web][keyserver] codeVersion -> 29 (authored by ashoat).
[web][keyserver] codeVersion -> 29
Wed, Sep 20, 10:42 AM
ashoat closed D9248: [native] Fix rescinds and badge-only notifs on iOS.
Wed, Sep 20, 10:40 AM
ashoat committed rCOMM4a1baebbbe0f: [native] Fix rescinds and badge-only notifs on iOS (authored by ashoat).
[native] Fix rescinds and badge-only notifs on iOS
Wed, Sep 20, 10:40 AM
ashoat added a comment to D9248: [native] Fix rescinds and badge-only notifs on iOS.

Nope, this doesn't relate to any keyserver change

Wed, Sep 20, 10:38 AM
ashoat accepted D9227: [patch] Patch `expo-modules-core` to unblock iOS build on macOS Sonoma.

Can you create a GitHub issue on the Expo repo and link it here before landing?

Wed, Sep 20, 10:36 AM
ashoat added a comment to D9225: [lib] Create function for extracing keyserver id from object id.

This does not handle pending threads (more precisely sidebars created from a message)

Wed, Sep 20, 6:17 AM
ashoat added a comment to D9005: [lib] Introduce chat mention utilities for plain text.

I'm no longer using entityTextToRawString since I only need to extract alternative text. Please take a look at the latest diff, 31258. Throughout the entire stack, I use the useResolvedThreadInfo hook.

Wed, Sep 20, 6:12 AM
ashoat accepted D9239: [keyserver] Prevent non-composable message types from being pinned on the keyserver.

One thing to note is that with ENG-4981, one of the short term solutions was to just disable the pin action for sidebar source messages.

Wed, Sep 20, 6:08 AM
ashoat accepted D9238: [lib] Extract the unsupported sidebar source message types to one single source of truth.

Accepting to unblock; we can dedup everything and figure out the types later. This Linear comment will hopefully be helpful

Wed, Sep 20, 6:01 AM
ashoat accepted D9237: [lib] Update useCanCreateSidebarFromMessage to prevent sidebar creation for unsupported message types.

Should we use isMessageSidebarSourceReactionEditOrPin for this instead, and extend it to include messageTypes.CHANGE_ROLE? We'll want to eventually unify all of these checks in one place (see here). That will probably be in the message specs, but that doesn't mean we'll delete isMessageSidebarSourceReactionEditOrPin – it can be a helper function that checks the message specs

Wed, Sep 20, 5:57 AM
ashoat requested changes to D9236: [keyserver] Update keyserver delete account endpoint to not require password.
  1. Please extend the test plan to include testing a request where the password field is not supplied. deleteAccountRequestInputValidator takes a password: t.maybe(tPassword), and it's not clear to me if that will crash when password is not supplied (as opposed to password: undefined or something)
  2. After this change, we no longer need a request passed to deleteAccount at all. While accountDeletionResponder will need to support a request for a while (for old clients), deleteAccount is downstream and should be abstracted from that
    • First, this will require some changes in this file so that request is no longer a param. See inline comments
    • Second, you'll want to update accountDeletionResponder to no longer pass request in
Wed, Sep 20, 5:52 AM
ashoat closed D9234: [keyserver] Skip early exit in deleteThread if ignorePermissions.
Wed, Sep 20, 5:28 AM
ashoat committed rCOMMd587cc151c26: [keyserver] Skip early exit in deleteThread if ignorePermissions (authored by ashoat).
[keyserver] Skip early exit in deleteThread if ignorePermissions
Wed, Sep 20, 5:28 AM

Tue, Sep 19

ashoat added a comment to D9235: [lib] introduce createPendingPersonalThread function.

No reviewers on this

Tue, Sep 19, 3:29 PM
ashoat added a comment to D9005: [lib] Introduce chat mention utilities for plain text.

Thought about this some more... I suspect you haven't given much thought to ENS names, and to properly handle it will need updates across the whole stack, from the search index, to the display of results, to the representation of the "markdown", to the final rendering of the tag.

Tue, Sep 19, 3:24 PM
ashoat added reviewers for D9234: [keyserver] Skip early exit in deleteThread if ignorePermissions: rohan, inka.
Tue, Sep 19, 3:20 PM
ashoat requested review of D9234: [keyserver] Skip early exit in deleteThread if ignorePermissions.
Tue, Sep 19, 2:49 PM
ashoat accepted D9233: [iOS] Add `withEnableSampleProfiling(true)` to `makeRuntimeConfig`.

Okay, please make sure that upstream React Native does this on all builds too before landing!

Tue, Sep 19, 1:50 PM
ashoat committed rCOMM79192f53a0a4: [web][keyserver] codeVersion -> 28 (authored by ashoat).
[web][keyserver] codeVersion -> 28
Tue, Sep 19, 1:44 PM
ashoat added a comment to D9233: [iOS] Add `withEnableSampleProfiling(true)` to `makeRuntimeConfig`.

It seems sketchy to do this in production, especially given it's undocumented. I'm worried about the potential impact on performance. Can we restrict this change to dev builds?

Tue, Sep 19, 1:43 PM
ashoat closed D9232: [keyserver] Prevent creation of sidebars from pinned message robotext.
Tue, Sep 19, 1:42 PM
ashoat committed rCOMM01bf7ede77a3: [keyserver] Prevent creation of sidebars from pinned message robotext (authored by ashoat).
[keyserver] Prevent creation of sidebars from pinned message robotext
Tue, Sep 19, 1:42 PM
ashoat closed D9230: [keyserver] Delete sidebars created from pinned message robotexts.
Tue, Sep 19, 1:42 PM
ashoat closed D9231: [keyserver] Move sidebar-of-sidebar check earlier in createThread.
Tue, Sep 19, 1:42 PM
ashoat committed rCOMM47a6b495b17c: [keyserver] Move sidebar-of-sidebar check earlier in createThread (authored by ashoat).
[keyserver] Move sidebar-of-sidebar check earlier in createThread
Tue, Sep 19, 1:42 PM
ashoat committed rCOMM44a1bbf93561: [keyserver] Delete sidebars created from pinned message robotexts (authored by ashoat).
[keyserver] Delete sidebars created from pinned message robotexts
Tue, Sep 19, 1:42 PM
ashoat closed D9229: [keyserver] Add ignorePermissions option to deleteThread.
Tue, Sep 19, 1:42 PM
ashoat committed rCOMMc4344888e795: [keyserver] Add ignorePermissions option to deleteThread (authored by ashoat).
[keyserver] Add ignorePermissions option to deleteThread
Tue, Sep 19, 1:42 PM
ashoat updated the diff for D9232: [keyserver] Prevent creation of sidebars from pinned message robotext.

Also prevent sidebar creation from a CHANGE_ROLE robotext. Context is in ENG-4981... this change won't solve the issue (since the client app crashes in the pending sidebar, before the sidebar creation can even go through) but it's probably a good idea

Tue, Sep 19, 1:40 PM
ashoat published D9232: [keyserver] Prevent creation of sidebars from pinned message robotext for review.
Tue, Sep 19, 1:32 PM
ashoat published D9231: [keyserver] Move sidebar-of-sidebar check earlier in createThread for review.
Tue, Sep 19, 1:32 PM
ashoat published D9230: [keyserver] Delete sidebars created from pinned message robotexts for review.
Tue, Sep 19, 1:31 PM
ashoat published D9229: [keyserver] Add ignorePermissions option to deleteThread for review.
Tue, Sep 19, 1:31 PM
ashoat committed rCOMMf8707d9b6d0d: [web][keyserver] codeVersion -> 27 (authored by ashoat).
[web][keyserver] codeVersion -> 27
Tue, Sep 19, 11:42 AM
ashoat accepted D9228: [iOS] Set `_LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION` flag to unblock iOS build.
Tue, Sep 19, 10:44 AM
ashoat added a comment to D9227: [patch] Patch `expo-modules-core` to unblock iOS build on macOS Sonoma.

Is there a GitHub issue in Expo's GitHub repo for this?

Tue, Sep 19, 10:43 AM
ashoat added a comment to D9218: [keyserver] Render raw chat mention in the notification text.

The diff title here should probably include the word "notification" somewhere :)

Tue, Sep 19, 7:38 AM
ashoat added a comment to D9005: [lib] Introduce chat mention utilities for plain text.

We shouldn't use entityTextToRawString directly. I'm not sure what the alternatives are here, and what the added complexity will be. Please investigate and get back to us

Tue, Sep 19, 7:31 AM
ashoat added a comment to D8944: [web] Modify mention typeahead regex.
In D8944#263772, @rohan wrote:

I think it'll be useful to add some unit tests for this regex, alongside the regex in D8834

Tue, Sep 19, 7:27 AM
ashoat updated subscribers of D8900: [lib] Add chat mention SearchIndex selector.

Is the performance concern pointed out by @tomek going to lead to additional React render cycles? If yes, we should prioritize addressing it. cc @atul

Tue, Sep 19, 7:25 AM
ashoat added a reviewer for D8897: [native] Refactor TypeaheadTooltip component: atul.

Adding @atul as a reviewer in case he has any comments about React performance here

Tue, Sep 19, 7:22 AM
ashoat closed D9222: [lib] Don't drop messages in MessageStore that don't have a corresponding ThreadInfo.
Tue, Sep 19, 6:21 AM
ashoat committed rCOMM736f8d5b50f6: [lib] Don't drop messages in MessageStore that don't have a corresponding… (authored by ashoat).
[lib] Don't drop messages in MessageStore that don't have a corresponding…
Tue, Sep 19, 6:21 AM

Mon, Sep 18

ashoat added inline comments to D9223: [nix] add logs to console while waiting for MariaDB to come up.
Mon, Sep 18, 2:32 PM
ashoat requested review of D9222: [lib] Don't drop messages in MessageStore that don't have a corresponding ThreadInfo.
Mon, Sep 18, 1:39 PM
ashoat added a comment to D9121: [lib] Create useKeyserverCall.

Don't forget to create one for the input validator idea too!

Mon, Sep 18, 8:12 AM
ashoat added inline comments to D9121: [lib] Create useKeyserverCall.
Mon, Sep 18, 6:53 AM
ashoat requested changes to D9204: [lib] introduce threadInfosSelectorForThreadType selector.

There's code that exists for the search experience that handles finding a thread for a given user if it already exists, OR creating a "pending" thread if that thread doesn't exist yet.

Mon, Sep 18, 6:49 AM
ashoat accepted D8730: Implement displaying error message notification for cases that might cause empty notification on Android.

Looks good, but would be great if @tomek could take a look, as he's more familiar with our Android notif code

Mon, Sep 18, 6:21 AM

Sat, Sep 16

ashoat accepted D9219: [native] Bump `react-native` to `0.70.9`.
Sat, Sep 16, 12:05 PM

Fri, Sep 15

ashoat accepted D9108: [web] Initialize database at startup on safari.

Nice thank you!!

Fri, Sep 15, 9:46 PM
ashoat added inline comments to D9121: [lib] Create useKeyserverCall.
Fri, Sep 15, 8:10 PM
ashoat added inline comments to D9205: [native] Introduce naive `ChatThreadListSearch` component.
Fri, Sep 15, 7:59 PM
ashoat added inline comments to D9205: [native] Introduce naive `ChatThreadListSearch` component.
Fri, Sep 15, 7:50 PM
ashoat added inline comments to D9205: [native] Introduce naive `ChatThreadListSearch` component.
Fri, Sep 15, 7:48 PM
ashoat added inline comments to D9206: [native] Move rest of Search views to `ChatThreadListSearch`.
Fri, Sep 15, 7:17 PM

Thu, Sep 14

ashoat accepted D8929: [keyserver] modify createPickledOlmSession.
Thu, Sep 14, 12:19 PM
ashoat added a comment to D9204: [lib] introduce threadInfosSelectorForThreadType selector.

How does the code for search results currently handle this? I think there's a similar need there for getting a "pending or realized" thread for the viewer and a given user. The new selector here makes me wonder if you're reusing as much of that code as possible.

Thu, Sep 14, 11:29 AM
ashoat accepted D8407: [keyserver] accountOwnershipResponder.
Thu, Sep 14, 11:26 AM
ashoat accepted D9154: [lib] Add uuid dependency.
Thu, Sep 14, 9:12 AM
ashoat accepted D9151: [native] Add expo-file-system dependency.
Thu, Sep 14, 8:32 AM
ashoat added a comment to D8901: [lib] Add userStoreMentionSearchIndex selector.

I can confirm that we're looking to do only prefix search for @-mentions of chats. We'll continue to do a "full text search" for in the "Search chats" experience as well as the message search experience. But the typeahead is meant to be prefix-only

Thu, Sep 14, 7:40 AM
ashoat added a reverting change for rCOMM8428f7fc38ae: Temporary changes for staff release: rCOMM18d58e18f1c4: Revert "Temporary changes for staff release".
Thu, Sep 14, 6:53 AM
ashoat committed rCOMM18d58e18f1c4: Revert "Temporary changes for staff release" (authored by ashoat).
Revert "Temporary changes for staff release"
Thu, Sep 14, 6:53 AM
ashoat committed rCOMM5fc4cdd061e2: [native] codeVersion -> 260 (authored by ashoat).
[native] codeVersion -> 260
Thu, Sep 14, 6:53 AM
ashoat committed rCOMM8428f7fc38ae: Temporary changes for staff release (authored by ashoat).
Temporary changes for staff release
Thu, Sep 14, 6:53 AM
ashoat committed rCOMMa59a0798997c: [native] codeVersion -> 259 (authored by ashoat).
[native] codeVersion -> 259
Thu, Sep 14, 6:51 AM
ashoat committed rCOMM9243eabdb853: [web][keyserver] codeVersion -> 26 (authored by ashoat).
[web][keyserver] codeVersion -> 26
Thu, Sep 14, 6:45 AM
ashoat closed D9201: [keyserver] Don't send empty ID cleanup query in notifs code.
Thu, Sep 14, 6:43 AM
ashoat committed rCOMMe8043a84cc5a: [keyserver] Don't send empty ID cleanup query in notifs code (authored by ashoat).
[keyserver] Don't send empty ID cleanup query in notifs code
Thu, Sep 14, 6:43 AM
ashoat published D9201: [keyserver] Don't send empty ID cleanup query in notifs code for review.

Publishing now, but will wait on CI before landing

Thu, Sep 14, 6:16 AM
ashoat closed D9186: [native] Avoid crashing Redux migration if inviteLinksStore isn't initialized.
Thu, Sep 14, 6:04 AM
ashoat committed rCOMM2f4d696e218b: [native] Avoid crashing Redux migration if inviteLinksStore isn't initialized (authored by ashoat).
[native] Avoid crashing Redux migration if inviteLinksStore isn't initialized
Thu, Sep 14, 6:04 AM
ashoat closed D9185: [native] Extract default Redux state to separate file.
Thu, Sep 14, 6:04 AM
ashoat committed rCOMM3cf8825bf256: [native] Extract default Redux state to separate file (authored by ashoat).
[native] Extract default Redux state to separate file
Thu, Sep 14, 6:04 AM

Wed, Sep 13

ashoat closed D9196: [native] Clean up after each Android Rust build in CI.
Wed, Sep 13, 6:05 PM
ashoat committed rCOMM2f41fe68f27b: [native] Clean up after each Android Rust build in CI (authored by ashoat).
[native] Clean up after each Android Rust build in CI
Wed, Sep 13, 6:05 PM
ashoat requested review of D9196: [native] Clean up after each Android Rust build in CI.
Wed, Sep 13, 2:28 PM
ashoat accepted D9190: [native] Replace `componentDidUpdate` with `useEffect`s in `ConnectedChatThreadList`.
Wed, Sep 13, 1:42 PM
ashoat accepted D9129: [lib] utility function to get array of OneTimeKeys from an OLMOneTimeKeys object.

Thanks!!

Wed, Sep 13, 12:24 PM
ashoat accepted D9189: [native] Lift `component[DidMount/WillUnmount]` lifecycle methods to `useEffect` in `ConnectedChatThreadList`.
Wed, Sep 13, 11:37 AM