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

Voting: avoid crashing when external app details are not loadable #1066

Merged
merged 4 commits into from
Jan 18, 2020

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Jan 14, 2020

Things like an app's name come from IPFS, and may not be available all the time.

The importance of re-spec'ing the data structures involved with apps is becoming clearer, as it'll let us more clearly mark whether something is always going to be available or not from aragon.js.

This can happen when the external app's artifacts aren't loadable. In
general we should prefer doing operations using the external app's
address, as this is always available.
Copy link
Contributor

@bpierre bpierre left a comment

Choose a reason for hiding this comment

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

LGTM!

The importance of re-spec'ing the data structures involved with apps is becoming clearer, as it'll let us more clearly mark whether something is always going to be available or not from aragon.js.

Agreed, do you think it should happen in the background script?

address: appAddress,
name,
// If the app name was not loaded, use the app's address
name: name || appAddress,
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we should use the shortened version here?

Copy link
Contributor Author

@sohkai sohkai Jan 18, 2020

Choose a reason for hiding this comment

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

Yes, let's do that! Done in ccb6e2b.

@sohkai
Copy link
Contributor Author

sohkai commented Jan 18, 2020

Agreed, do you think it should happen in the background script?

This will need to happen starting from @aragon/wrapper and will propagate to @aragon/api.

@sohkai sohkai merged commit 0f300f6 into master Jan 18, 2020
@sohkai sohkai deleted the voting-fix-missing-app-details branch January 18, 2020 23:38
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
…agon#1066)

The external app details are not available when we can't load the the external app's artifacts. In general we should prefer doing operations using the external app's address, as this will always be available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants