Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] fetch all replies #8

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/javascript/flavours/glitch/actions/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ export function clearSearch() {
};
};

export function submitSearch() {
export function submitSearch(search) {
return (dispatch, getState) => {
const value = getState().getIn(['search', 'value']);
const value = search ? search : getState().getIn(['search', 'value']);
const signedIn = !!getState().getIn(['meta', 'me']);

if (value.length === 0) {
Expand Down
65 changes: 65 additions & 0 deletions app/javascript/flavours/glitch/actions/statuses.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import api from '../api';
import { deleteFromTimelines } from './timelines';
import { importFetchedStatus, importFetchedStatuses } from './importer';
import { ensureComposeIsVisible, setComposeToStatus } from './compose';
import { submitSearch } from './search';
import { showAlert } from './alerts';

export const STATUS_FETCH_REQUEST = 'STATUS_FETCH_REQUEST';
export const STATUS_FETCH_SUCCESS = 'STATUS_FETCH_SUCCESS';
Expand All @@ -16,6 +18,10 @@ export const CONTEXT_FETCH_REQUEST = 'CONTEXT_FETCH_REQUEST';
export const CONTEXT_FETCH_SUCCESS = 'CONTEXT_FETCH_SUCCESS';
export const CONTEXT_FETCH_FAIL = 'CONTEXT_FETCH_FAIL';

export const EXTERNAL_CONTEXT_FETCH_REQUEST = 'EXTERNAL_CONTEXT_FETCH_REQUEST';
export const EXTERNAL_CONTEXT_FETCH_SUCCESS = 'EXTERNAL_CONTEXT_FETCH_SUCCESS';
export const EXTERNAL_CONTEXT_FETCH_FAIL = 'EXTERNAL_CONTEXT_FETCH_FAIL';

export const STATUS_MUTE_REQUEST = 'STATUS_MUTE_REQUEST';
export const STATUS_MUTE_SUCCESS = 'STATUS_MUTE_SUCCESS';
export const STATUS_MUTE_FAIL = 'STATUS_MUTE_FAIL';
Expand Down Expand Up @@ -219,6 +225,65 @@ export function fetchContextFail(id, error) {
};
};



export function fetchExternalContext(status_url, id){
// Get just the protocol and domain of the instance
// note that we are just assuming that the api is at <domain>/api
// since I don't know a better way of finding it
let { origin, pathname } = new URL(status_url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could maybe add examples here in the comments for newbies like me? :) Like: status_url = "https://origin/@user_id/external_id" where user_id is the OP's user id on the origin instance and external_id is the OP's status id? (dunno if I've got this right tho, lol)

let paths = pathname.split('/');
let external_id = paths[paths.length - 1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering is external_id = id also true here?? based on app/javascript/flavours/glitch/features/status/index.js#L470 : this.props.dispatch(fetchExternalContext(status.get('url'), statusId));

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no i don't think so, each instance makes its own local IDs for posts that it uses internally which don't necessarily match the Ids used by other instances.


return (dispatch, getState) => {
dispatch(fetchExternalContextRequest(id));

api(getState).get(`${origin}/api/v1/statuses/${external_id}/context`).then(response => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do a check here whether the protocol property of ${origin} contains https or not? just for security to avoid sending a request to an http server?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToDo: we'd need to add handlers for non-mastodon softwares

dispatch(showAlert('Getting Replies...', `Getting ${response.data.descendants.length} Replies.`));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo: We could maybe make this alert more accessible? It is currently in English. If possible, we could enable translation into other languages in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I gotta see how the translation system works and then I'll change

response.data.descendants.forEach((status) => {
dispatch(submitSearch(status.url));
});
// dispatch(importFetchedStatuses(response.data.ancestors.concat(response.data.descendants)));
dispatch(fetchExternalContextSuccess(id, response.data.ancestors, response.data.descendants));

}).catch(error => {
if (error.response && error.response.status === 404) {
// dispatch(deleteFromTimelines(id));
}

dispatch(fetchContextFail(id, error));
});
};
}


export function fetchExternalContextRequest(id) {
return {
type: EXTERNAL_CONTEXT_FETCH_REQUEST,
id,
};
};

export function fetchExternalContextSuccess(id, ancestors, descendants) {
console.log('fetch success');
return {
type: EXTERNAL_CONTEXT_FETCH_SUCCESS,
id,
ancestors,
descendants,
statuses: ancestors.concat(descendants),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we concat only those descendants where visibility = public? not sure what the guidelines are for statuses that have opted out of discovery (e.g where visibility = unlisted)?

};
};

export function fetchExternalContextFail(id, error) {
return {
type: EXTERNAL_CONTEXT_FETCH_FAIL,
id,
error,
skipAlert: true,
};
};

export function muteStatus(id) {
return (dispatch, getState) => {
dispatch(muteStatusRequest(id));
Expand Down
26 changes: 26 additions & 0 deletions app/javascript/flavours/glitch/features/status/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { createSelector } from 'reselect';
import { fetchStatus } from 'flavours/glitch/actions/statuses';
import MissingIndicator from 'flavours/glitch/components/missing_indicator';
import LoadingIndicator from 'flavours/glitch/components/loading_indicator';
import Button from 'flavours/glitch/components/button';
import DetailedStatus from './components/detailed_status';
import ActionBar from './components/action_bar';
import Column from 'flavours/glitch/features/ui/components/column';
Expand Down Expand Up @@ -36,6 +37,7 @@ import {
revealStatus,
translateStatus,
undoStatusTranslation,
fetchExternalContext,
} from 'flavours/glitch/actions/statuses';
import { initMuteModal } from 'flavours/glitch/actions/mutes';
import { initBlockModal } from 'flavours/glitch/actions/blocks';
Expand Down Expand Up @@ -463,6 +465,12 @@ class Status extends ImmutablePureComponent {
this.props.dispatch(openModal('EMBED', { url: status.get('url') }));
}

handleFetchContext = () => {
const { status, statusId } = this.props;
this.props.dispatch(fetchExternalContext(status.get('url'), statusId));
this.forceUpdate();
}

handleHotkeyToggleSensitive = () => {
this.handleToggleMediaVisibility();
}
Expand Down Expand Up @@ -707,6 +715,24 @@ class Status extends ImmutablePureComponent {
</HotKeys>

{descendants}

{isLocal
? null
: <div style={{
display: 'block',
margin: '1em 1em',
}}
>
<Button
className={'primary'}
text={'Get More Replies'}
title={'Get More Replies'}
onClick={this.handleFetchContext}
block
/>
</div>
}

</div>
</ScrollContainer>

Expand Down
10 changes: 10 additions & 0 deletions app/javascript/flavours/glitch/reducers/contexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { CONTEXT_FETCH_SUCCESS } from 'flavours/glitch/actions/statuses';
import { TIMELINE_DELETE, TIMELINE_UPDATE } from 'flavours/glitch/actions/timelines';
import { Map as ImmutableMap, List as ImmutableList } from 'immutable';
import compareId from '../compare_id';
import { SEARCH_FETCH_SUCCESS } from '../actions/search';

const initialState = ImmutableMap({
inReplyTos: ImmutableMap(),
Expand Down Expand Up @@ -88,6 +89,13 @@ const updateContext = (state, status) => {
return state;
};

const updateContexts = (state, statuses) => {
statuses.forEach((status) => {
state = updateContext(state, status);
});
return state;
};

export default function replies(state = initialState, action) {
switch(action.type) {
case ACCOUNT_BLOCK_SUCCESS:
Expand All @@ -99,6 +107,8 @@ export default function replies(state = initialState, action) {
return deleteFromContexts(state, [action.id]);
case TIMELINE_UPDATE:
return updateContext(state, action.status);
case SEARCH_FETCH_SUCCESS: //STATUSES_IMPORT: // eg. from external context expansion
return updateContexts(state, action.results.statuses);
default:
return state;
}
Expand Down