message_actions: Add move message option #5189

SilentCruzer · 2022-01-09T11:04:07Z

Closes: part of #5165

Changes:

Created a new component called MoveMessage to display the details on where to move the message. The screen changes slightly depending on whether the user is admin or not. If the user is an admin, they can change the stream of the message, if not admin they can only change the topic in the current stream.
Added navigation to the above screen.
Updated the update message API, added a new param called stream_id and changes the type of propagate_mode from boolean to string according to the API documentation.
Added the propagate_mode to allow users to choose the option to move one message/ later messages / previous and following messages.

Changes video:

move_message.mp4

Admin user UI (the stream part becomes a drop down):

gnprice

Thanks @SilentCruzer! Here's a partial review, with the things I had time to get to tonight. (There's more logic I didn't get to, especially the bottom half of the MoveMessage component's definition.)

Please take a look and make changes, and don't hesitate to ask questions in #mobile-team for anything you're unsure about.

gnprice · 2022-01-11T06:30:10Z

src/nav/navActions.js

@@ -125,6 +125,17 @@ export const navigateToMessageReactionScreen = (
  reactionName?: string,
 ): GenericNavigationAction => StackActions.push('message-reactions', { messageId, reactionName });

+export const navigateToMoveMessage = (
+  message: Message | Outbox,


This type needs to agree with the route: RouteProp<…> on the component.

(This is one of the rare places where the type-checker isn't able to check this sort of thing for you, because of limitations in the types in the react-navigation library: #4757 (comment) .)

gnprice · 2022-01-11T06:32:48Z

src/message/moveMessage.js

+  }),
+};
+
+export default function MoveMessage(props: Props): Node {


Component names are capitalized in our code, following normal React convention: search for "capital" in https://reactjs.org/docs/components-and-props.html .

The local name on this line, MoveMessage, is capitalized correctly. The module / filename should be capitalized the same way.

gnprice · 2022-01-11T06:34:11Z

src/message/moveMessage.js

+  const cardColor = themeContext.cardColor;
+  const auth = useSelector(getAuth);
+  const streams = useSelector(getStreams);
+  const isadmin = props.route.params.isadmin;


Make the word boundary visible: isAdmin, not isadmin (for both the local and the property).

gnprice · 2022-01-11T06:36:34Z

src/message/moveMessage.js

+  const auth = useSelector(getAuth);
+  const streams = useSelector(getStreams);
+  const isadmin = props.route.params.isadmin;
+  const id = props.route.params.message.id;


id is too broad -- where it's used later in the code, it makes it hard to see what kind of thing it's identifying. Better would be messageId. Or just message, and say message.id where it's used.

gnprice · 2022-01-11T06:36:40Z

src/message/moveMessage.js

+  const id = props.route.params.message.id;
+  const names = [];
+  const [subject, setSubject] = useState(props.route.params.message.subject);
+  const [propagate_mode, setPropagateMode] = useState('javachange_one');


"java"? I think this value isn't what you intended.

This also points out a need for better type-checking here, by writing down more specific types: we don't actually want api.updateMessage to allow just any string for the propagate_mode it gets passed. With better types there, Flow would be able to point out that this line is wrong.

To do that, make a preparatory commit (which just means a commit early in the branch, before the main commit) that edits src/api/messages/updateMessage.js to give a more precise type for propagate_mode. Refer to the server API documentation, at the page linked to in that function's jsdoc, for what the type should be.

Note that in Flow if you have a value that should be one of a fixed list of strings, like "either foo or bar", you can spell that type like 'foo' | 'bar'. See docs: https://flow.org/en/docs/types/unions/#toc-union-type-syntax

gnprice · 2022-01-11T06:55:31Z

src/message/moveMessage.js

+    autocompleteWrapper: {
+      position: 'absolute',
+      top: 0,
+      width: '100%',
+    },
+    composeText: {
+      flex: 1,


Some of these styles don't seem to be actually used in the code. Should delete them so it's easier to see the ones that do matter.

gnprice · 2022-01-11T06:57:16Z

src/message/moveMessage.js

+  const handleTopicChange = (Topic: string) => {
+    setSubject(Topic);


Variable names start with lower case. (Except if their value is a React component type, or a class (not just an instance of a class, but a class itself), both of which are rare.)

gnprice · 2022-01-11T07:01:03Z

src/message/moveMessage.js

+  const updateTextInput = (textInput, text) => {
+    if (textInput === null) {
+      // Depending on the lifecycle events this function is called from,
+      // this might not be set yet.
+      return;


This comment appears to be copy-pasted from ComposeBox.js. Is the comment true here? What does it mean?

… Reading a bit more, it looks like in fact textInput will always be null here -- it comes from topicInputRef.current, and we never set topicInputRef or pass it somewhere that could end up setting it. So this function never does anything.

gnprice · 2022-01-11T07:12:45Z

src/action-sheets/index.js

+    moveMessage.narrow = narrow;
+    moveMessage.admin = ownUser.is_admin;


This appears to be setting properties on the function (which is a global constant for this module), in order to store some of the data that we're currently processing.

Do not do this. This is a brittle pattern -- effectively you're storing these values as global variables.

Instead, use the mechanisms that the surrounding code already uses to pass the data that comparable functions need. Here, moveMessage needs to be a Button<MessageArgs>, because that's what's in the array that constructMessageActionButtons returns. Looking at the definition of Button<Args>, in general the function takes an Args -- so for Button<MessageArgs> it takes a MessageArgs. So we just need the relevant information to be added to MessageArgs.

Specifically we need narrow there. For ownUser.is_admin, the moveMessage callback doesn't really need that information -- the MoveMessage component will need it, but it can get that information from Redux for itself.

BTW in general if you see a way to make the code work that involves structuring it with a bad pattern, and you can't find a cleaner way to do it, it's OK to send a PR where it uses the bad structure (especially when it's a small part of the PR's code, like here) -- but it's important to call attention to it clearly. Typically that might mean

both a TODO comment in the code,

and a prominent note about it in the PR description, or as a PR comment shortly after sending the PR.

That way you help the reviewer make sure to notice it so they can offer suggestions on how to handle the problem.

Better yet, typically, is to ask in chat about how to handle the thing you're trying to do, before you have the PR ready to send. But sometimes the easiest way to make the question clear is with the context of the rest of the code, and then a PR with clear communication about the question is a good way to go.

gnprice · 2022-01-11T07:30:56Z

src/message/moveMessage.js

+  };
+
+  const updateMessage = async () => {
+    const stream_info = await api.getStreamId(auth, stream);


This will go and make a request to the server to look up what stream has the given name.

We don't want to do that. We already have data on all the streams the user has access to know about, and that's the data we used for presenting the UI the user will have used to select the stream. We should use that data instead.

That's better because not only does it make things faster, and more reliable on a weak network connection -- because we save a request round-trip -- but also it keeps the action we actually take aligned with the action the user chose, eliminating a class of bugs where there was a mismatch between the data we had at the moment the user chose something in the UI and the data the server has by the time it handles the request.

For example, if I go to move something to stream #awesome-team, and right around that moment someone else goes and renames #awesome-team to #awesome team, things should just work -- the messages should get moved to that stream, even though its name was #awesome team by the time the request made it to the server.

(This connects to the point above about using stream IDs rather than stream names.)

SilentCruzer · 2022-01-21T21:14:05Z

src/message/MoveMessage.js

+    setNarrow(streamNarrow(String(pickedStreamName)));
+  };
+
+  const handlePropagateMode = (propagatePickerIndex: number) => {


handlePropagateMode

So this function is to handle the propagate model while using the Picker component.

After I added the types for the propagate_mode in updateMessage API, the setPropagateMode function does not work with onValueChange function of the Picker component(line 202).

Since there will be only three values, I used if-else statements based on the values of the index

This works, but is fragile: these conditions in handlePropagateMode now have to line up with the order of options in the picker, and they're separated by quite a bit of code. It'd be easy for them to get out of sync, so that the items no longer had the effects they're labelled for.

For that reason it's better to use the value feature of Picker / PickerItem.

If you get errors from Flow but you're confident that what actually happens with the data is correct, you can add a $FlowFixMe comment with an explanation of why the actual behavior is correct.

SilentCruzer · 2022-01-21T21:19:59Z

src/message/MoveMessage.js

+    Toast.show('Moved Message');
+  };
+
+  const handleNarrow = (pickedStreamId: number) => {


handleNarrow

Noticed a bug when I made the PR. When move message function was accessed through the topic narrow, the topic auto complete didn't suggest any topic. So I had to create a new stream narrow.

To create a stream narrow, I needed the name of the stream.

First idea was to use the label I added to the items in the Picker component, but I later realized that I can't access the label after adding it, so I used the below method.

Since I had the id of the stream, mapped through the current stream list, and found the name,

Cool, looking up the stream name given the stream ID is the right direction.

Don't do it with a linear search through the whole list of streams, though: #3339.

Instead, use getStreamsById, similar to the getStreamsByName I mentioned in another comment.

SilentCruzer · 2022-02-17T14:50:51Z

@gnprice , can you pls review the latest changes?

gnprice · 2022-04-15T03:16:12Z

src/message/MoveMessage.js

+  const isAdmin = useSelector(getOwnUser).is_admin;
+  const messageId = props.route.params.message.id;
+  const currentStreamName = streamNameOfNarrow(props.route.params.messageNarrow);
+  const currentStreamId = allStreams.find(s => s.name === currentStreamName)?.stream_id;


Avoid linear searches like this one: #3339

That's why at #5189 (comment) I recommended getStreamsByName, which is a data structure for accomplishing the same lookup efficiently.

gnprice · 2022-04-15T03:19:46Z

src/message/MoveMessage.js

+      api
+        .updateMessage(
+          auth,
+          { subject, stream_id: streamId, propagate_mode: propagateMode },
+          messageId,
+        )
+        .catch(error => {


Don't use Promise#catch; instead, make the surrounding function async, and use await inside a try/catch block. (You can search for await api to find a number of examples.)

The reason is that with Promise#catch it's hard to avoid having subtle edge cases where exceptions don't get handled properly, and using try/catch blocks instead gives a much more robust structure for managing our exception handling.

(This isn't something that's obvious a priori. I wish I had a good writeup of it to link you to, but I don't.)

Oh, and here's a related benefit of using try/catch: you have these two .catch calls that are completely identical, and it would be better to say that just once. With try/catch, that's very straightforward to do: just put the try/catch outside the if/else that controls the two different paths.

gnprice · 2022-04-15T03:23:52Z

src/message/MoveMessage.js

+        .catch(error => {
+          showErrorAlert('Failed to move message', error.message);
+          props.navigation.goBack();
+        });
+    }
+    props.navigation.goBack();
+    Toast.show('Moved Message');


Ah, here's an example of how Promise#catch tends to lead to confused error-handling, which is easier to see and avoid when using try/catch.

This code will unconditionally show a toast with "Moved Message", even if the move actually ends up failing.

Instead, we should show a confirmation message like that only after we actually succeed.

gnprice · 2022-04-15T03:24:51Z

src/message/MoveMessage.js

+          showErrorAlert('Failed to move message', error.message);
+          props.navigation.goBack();
+        });
+    }
+    props.navigation.goBack();
+    Toast.show('Moved Message');


Both the error message and the success message should be translated. Search for showErrorAlert to see examples of how it's done at other call sites. See https://github.com/zulip/zulip-mobile/blob/main/docs/howto/translations.md#all-code-contributors-wiring-our-code-for-translations for background.

gnprice · 2022-04-15T03:27:52Z

src/message/MoveMessage.js

+          showErrorAlert('Failed to move message', error.message);
+          props.navigation.goBack();
+        });
+    }
+    props.navigation.goBack();
+    Toast.show('Moved Message');


Write the confirmation message in sentence case: only the first word capitalized, so like "Moved message".

gnprice · 2022-04-15T03:34:58Z

src/message/MoveMessage.js

+    setNarrow(streamNarrow(String(pickedStreamName)));
+  };
+
+  const handlePropagateMode = (propagatePickerIndex: number) => {


This works, but is fragile: these conditions in handlePropagateMode now have to line up with the order of options in the picker, and they're separated by quite a bit of code. It'd be easy for them to get out of sync, so that the items no longer had the effects they're labelled for.

For that reason it's better to use the value feature of Picker / PickerItem.

If you get errors from Flow but you're confident that what actually happens with the data is correct, you can add a $FlowFixMe comment with an explanation of why the actual behavior is correct.

gnprice · 2022-04-15T03:41:16Z

Thanks @SilentCruzer for the revision! Comments above. There've also been a few changes upstream which may generate conflicts you'll need to resolve when rebasing: in particular narrows now contain a stream ID instead of a stream name, and the action-sheets code has a more normal structure.

The UI looks pretty reasonable to me from those screenshots and video (thanks!), but I'd be curious if @alya has any feedback to add.

alya · 2022-04-15T05:48:01Z

Thanks for picking this up @SilentCruzer !

On the UI, is there a reason to order the parts of the UI differently from the web app? I would put the message content last, unless there's good reason not to.

I guess the notification options are not implemented here? That seems fine for an initial version, but we should make sure to keep track of that as a follow-up when this PR is merged.

alya · 2022-04-15T05:49:29Z

The other key part to note is that the permissions are much more complicated than described at the top of the PR. This is not an accurate summary:

If the user is an admin, they can change the stream of the message, if not admin they can only change the topic in the current stream.

We may want to wait for zulip/zulip#21739 to be resolved before we finish this feature?

alya · 2022-04-15T05:52:40Z

The other visual question I have is whether we have other buttons that look like this? E.g. the buttons in the profile settings UI look like quite different.

gnprice · 2022-04-15T20:15:07Z

The other key part to note is that the permissions are much more complicated than described at the top of the PR. This is not an accurate summary:

Hmmm, good point. I didn't look closely enough and missed that.

We may want to wait for zulip/zulip#21739 to be resolved before we finish this feature?

Yeah, we probably want to wait for that before attempting to write down the full permissions logic here. Probably also will want the permissions logic to be in shared code -- similar to at #5322 (comment) and the comments after that.

Do you think there would be value in having a version of this feature sooner that, say, was only available when you're an admin? If so, then the permissions logic for just that case may be relatively simple. Then if we merge a version of this UI that we only make available for admins, we can extend it later after we complete that revamp of how these permissions work and also refactor the logic for them into being shared.

I agree with all of Alya's other comments, too.

alya · 2022-04-20T07:41:25Z

Do you think there would be value in having a version of this feature sooner that, say, was only available when you're an admin? If so, then the permissions logic for just that case may be relatively simple. Then if we merge a version of this UI that we only make available for admins, we can extend it later after we complete that revamp of how these permissions work and also refactor the logic for them into being shared.

Yes, I think merging this feature just for admins and then extending it sounds like a great plan! Admins should always be able to edit the message topic and stream.

SilentCruzer · 2022-05-03T12:38:09Z

I have made the suggested changes and updated the PR.

@alya , I have moved the content part to the end and also changed the style of the button to match the other screens.(In the below screenshot, it is the same style used in the 'set a status' screen)

Screenshots:

gnprice

Thanks @SilentCruzer for the revision! Comments below.

gnprice · 2022-11-09T04:20:39Z

src/message/MoveMessage.js

+    try {
+      if (isAdmin) {
+        api.updateMessage(auth, messageId, {


These api.foo calls need await.

As discussed here:
#5189 (comment)
search for await api (e.g., use git grep -C2 "await api") to find examples.

Without that, the try/catch basically has no effect. You can demonstrate that by editing the implementation of api.updateMessage to artificially add an error, like throw new Error("oops"); or null.asdf;.

gnprice · 2022-11-09T04:23:01Z

src/message/MoveMessage.js

+      if (isAdmin) {
+        api.updateMessage(auth, messageId, {
+          subject,
+          stream_id: streamId,
+          propagate_mode: propagateMode,
+        });
+      } else {
+        api.updateMessage(auth, messageId, { subject, propagate_mode: propagateMode });


These would be clearer by unifying as one api.updateMessage call, because almost everything is the same in the two cases.

Instead, use a conditional just to control the one thing that changed. One handy trick for this, if you want an extra property in one case and not the other, is to spread a conditional expression:

{ subject, ...(isAdmin ? { stream_id: streamId } : {}), propagate_mode: propagateMode, }

gnprice · 2022-11-09T04:30:53Z

src/message/MoveMessage.js

+        </View>
+        <Text style={{ fontSize: 14, marginBottom: 10, color: 'gray' }}>Content:</Text>
+        <Text style={[styles.textColor, { marginBottom: 20 }]}>
+          {props.route.params.message.content.replace(/<(?:.|\n)*?>/gm, '')}


Let's factor this expression out as a local variable in the function.

That way it gets a name that helps explain what its intended meaning is. It's kind of opaque as it is.

Hmmm, is the idea of this that we're taking the HTML of the message and attempting to turn it into plain text? I don't think this strategy is going to work for making something readable, in general, for complex messages.

Instead of attempting to turn the HTML into readable text, I think a more reliable strategy will be to use the message's Markdown content.

We can get that from the server using api.getRawMessageContent. See editMessage in src/action-sheets/index.js for an example.

gnprice · 2022-11-09T04:40:18Z

src/message/MoveMessage.js

+            // $FlowFixMe[incompatible-call] : the itemValue will always be one of these values - change_one | change_later | change_all
+            onValueChange={(itemValue, itemIndex) => setPropagateMode(itemValue)}


Cool, this fixme comment is helpful and this code is clearer (cf #5189 (comment) on the previous version.)

Let's do two things to help clean this up further:

Let's take some of the information that's in that comment, and move it into code: write a cast, like setPropagateMode((itemValue: 'change_one' | 'change_later' | 'change_all')).

Flow will still give an error and require a fixme. But now the code itself already expresses the list of possible values we think it can be. So the fixme comment is now free to explain why -- namely, because those are the values in the items right below this.

gnprice · 2022-11-09T04:44:17Z

src/message/MoveMessage.js

+import type { Node } from 'react';
+import { ThemeContext, BRAND_COLOR } from '../styles';


nit: blank line between external and internal imports

gnprice · 2022-11-09T04:51:15Z

src/message/MoveMessage.js

+            />
+          </Picker>
+        </View>
+        <Text style={{ fontSize: 14, marginBottom: 10, color: 'gray' }}>Content:</Text>


Hmmm this doesn't get translated, does it.

Use ZulipTextIntl for this sort of UI text. If you search the codebase for <Text, you'll notice we have very few places using Text directly.

That will also ensure the text gets an appropriate style. In particular, looking back at the screenshots in dark mode I see that the contrast is pretty low -- the text is harder to read than it should be. ZulipTextIntl will get the text color from the theme, which means it'll get an appropriate color to contrast with the theme's background color.

gnprice · 2022-11-09T04:58:23Z

src/message/MoveMessage.js

+        <View style={styles.viewTitle}>
+          <TouchableOpacity onPress={() => props.navigation.goBack()}>
+            <Icon size={20} color="gray" name={iconName} />
+          </TouchableOpacity>
+          <Text style={styles.title}>Move Message</Text>
+          <View />
+        </View>


Don't try to make this sort of custom replacement for common widgets that exist across the app. Instead, use the same standard widgets that the rest of the app uses.

You can see the consequences in your screenshots at the top of the thread. The "go back" arrow here doesn't match what appears in the rest of the app:

Similarly, having the screen's title in the middle of the app bar is inconsistent with the rest of the app, which has it at the left.

To find the right pattern to use, find any other screen in the app that has the piece of UI you're looking for -- so here, an app bar with a go-back button -- and look at the code for that screen.

gnprice · 2022-11-09T05:00:54Z

src/message/MoveMessage.js

+export default function MoveMessage(props: Props): Node {
+  const themeContext = useContext(ThemeContext);
+  const backgroundColor = themeContext.backgroundColor;
+  const cardColor = themeContext.cardColor;
+  const iconName = Platform.OS === 'android' ? 'arrow-left' : 'chevron-left';
+  const auth = useSelector(getAuth);
+  const allStreams = useSelector(getStreams);
+  const isAdmin = useSelector(getOwnUser).is_admin;
+  const messageId = props.route.params.message.id;
+  const currentStreamId = streamIdOfNarrow(props.route.params.messageNarrow);
+  const currentStreamName = useSelector(state => getStreamForId(state, currentStreamId)).name;
+  const [narrow, setNarrow] = useState(streamNarrow(currentStreamId));
+  const [subject, setSubject] = useState(props.route.params.message.subject);
+  const [propagateMode, setPropagateMode] = useState('change_one');
+  const [streamId, setStreamId] = useState(currentStreamId);
+  const [topicFocus, setTopicFocus] = useState(false);
+  const _ = useContext(TranslationContext);
+


nit: Organize this by grouping together different kinds of definitions, with blank lines to separate groups.

Follow the patterns seen in other components in the codebase: first props, then context, then selectors, then state.

gnprice · 2022-11-09T05:02:41Z

src/message/MoveMessage.js

+  const currentStreamId = streamIdOfNarrow(props.route.params.messageNarrow);
+  const currentStreamName = useSelector(state => getStreamForId(state, currentStreamId)).name;
+  const [narrow, setNarrow] = useState(streamNarrow(currentStreamId));
+  const [subject, setSubject] = useState(props.route.params.message.subject);


These and some other spots would be made a bit simpler to read by pulling the route params out as locals:

const { message, messageNarrow } = props.route.params;

That also replaces the need for a messageId local -- can just say message.id.

gnprice · 2022-11-09T05:05:15Z

src/message/MoveMessage.js

+
+type Props = $ReadOnly<{|
+  navigation: AppNavigationProp<'move-message'>,
+  route: RouteProp<'move-message', {| message: Message | Outbox, messageNarrow: Narrow |}>,


What's the intended meaning of messageNarrow? This could use some jsdoc here to clarify.

I'm not sure what it's supposed to mean here. That in turn is necessary in order to properly review the logic below that consumes it.

SilentCruzer force-pushed the moveTopic branch 2 times, most recently from bd340e8 to 139c454 Compare January 9, 2022 13:04

gnprice reviewed Jan 11, 2022

View reviewed changes

SilentCruzer force-pushed the moveTopic branch from 139c454 to 133c7e0 Compare January 21, 2022 20:58

SilentCruzer commented Jan 21, 2022

View reviewed changes

SilentCruzer requested a review from gnprice January 21, 2022 21:21

gnprice reviewed Apr 15, 2022

View reviewed changes

alya mentioned this pull request Apr 20, 2022

Revamp topic editing permissions zulip/zulip#21739

Closed

gnprice mentioned this pull request Apr 22, 2022

stream action buttons: Implement search messages in stream #5248

Open

api.updateMessage: Add params stream_id and propagate_mode

beb2a52

SilentCruzer force-pushed the moveTopic branch from 133c7e0 to fdb2ecf Compare May 3, 2022 08:53

message_actions: Add move message option

d217768

SilentCruzer force-pushed the moveTopic branch from fdb2ecf to d217768 Compare May 3, 2022 12:24

gnprice reviewed Nov 9, 2022

View reviewed changes

		const handleTopicChange = (Topic: string) => {
		setSubject(Topic);

		moveMessage.narrow = narrow;
		moveMessage.admin = ownUser.is_admin;

		// $FlowFixMe[incompatible-call] : the itemValue will always be one of these values - change_one \| change_later \| change_all
		onValueChange={(itemValue, itemIndex) => setPropagateMode(itemValue)}

		import type { Node } from 'react';
		import { ThemeContext, BRAND_COLOR } from '../styles';

message_actions: Add move message option #5189

Are you sure you want to change the base?

message_actions: Add move message option #5189

Conversation

SilentCruzer commented Jan 9, 2022

Changes:

Changes video:

Admin user UI (the stream part becomes a drop down):

gnprice 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

SilentCruzer Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SilentCruzer Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

gnprice Apr 15, 2022 • edited Loading

Choose a reason for hiding this comment

SilentCruzer commented Feb 17, 2022

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

gnprice commented Apr 15, 2022

alya commented Apr 15, 2022

alya commented Apr 15, 2022 • edited Loading

alya commented Apr 15, 2022

gnprice commented Apr 15, 2022 • edited Loading

alya commented Apr 20, 2022 • edited Loading

SilentCruzer commented May 3, 2022

gnprice 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

SilentCruzer Jan 21, 2022 •

edited

Loading

SilentCruzer Jan 21, 2022 •

edited

Loading

gnprice Apr 15, 2022 •

edited

Loading

alya commented Apr 15, 2022 •

edited

Loading

gnprice commented Apr 15, 2022 •

edited

Loading

alya commented Apr 20, 2022 •

edited

Loading