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

Import CID #1128

Merged
merged 13 commits into from
Jun 15, 2021
Merged

Import CID #1128

merged 13 commits into from
Jun 15, 2021

Conversation

RyRy79261
Copy link
Contributor

@RyRy79261 RyRy79261 commented Jun 11, 2021

closes #1105

Getting an error when executing the add pin, not sure if that's expected or not
edit (tbaut): solved

@RyRy79261 RyRy79261 added Status: Review Needed 👀 Added to PRs when they need more review Type: Feature Added to PRs to identify that the change is a new feature. labels Jun 11, 2021
@render
Copy link

render bot commented Jun 11, 2021

@render
Copy link

render bot commented Jun 11, 2021

@Tbaut
Copy link
Collaborator

Tbaut commented Jun 14, 2021

Getting an error when executing the add pin, not sure if that's expected or not

Definitely not expected, this came from an error in the api-spec. Fixed it and published it, this is now sorted 🎉

I added a pin refresh once the pin is successfully added, but the UI is too quick, and actually refreshes too quickly so that the pin you just added doesn't show up and you still need a manual refresh of the page, which is annoying. Do you have an idea @FSM1 or @tanmoyAtb how to work around that? Should we listen to the upload callback in the call or something similar (as you can tell, I've no clue what I'm talking about :D )

@tanmoyAtb
Copy link
Contributor

Getting an error when executing the add pin, not sure if that's expected or not

Definitely not expected, this came from an error in the api-spec. Fixed it and published it, this is now sorted 🎉

I added a pin refresh once the pin is successfully added, but the UI is too quick, and actually refreshes too quickly so that the pin you just added doesn't show up and you still need a manual refresh of the page, which is annoying. Do you have an idea @FSM1 or @tanmoyAtb how to work around that? Should we listen to the upload callback in the call or something similar (as you can tell, I've no clue what I'm talking about :D )

From what I see, the refresh gets called, only after the add pin API has responded, don't really see a way of getting around it,
The add pin API returns an object with status queued.

Currently the pins API only returns pins with pinned status, the queued pins don't get returned, maybe we can have pins of all statuses, then the user would get a better sense of whats happening. 🤔

@Tbaut
Copy link
Collaborator

Tbaut commented Jun 14, 2021

Thanks Tanmoy, I thought we weren't filtering anything. Would you mind pointing me to where you found it, I couldn't find anything?

@tanmoyAtb
Copy link
Contributor

Thanks Tanmoy, I thought we weren't filtering anything. Would you mind pointing me to where you found it, I couldn't find anything?

We aren't filtering anything on the front end, the API is I think,
I printed the response from addPin api call here,
Screenshot 2021-06-14 at 11 46 49 PM

the response shows the pinning is queued
Screenshot 2021-06-14 at 11 49 48 PM

@Tbaut
Copy link
Collaborator

Tbaut commented Jun 15, 2021

ok here we go, we have now a status column and we show all the pins (except the failed ones)

image

Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Looks great 👍

@Tbaut Tbaut removed the Status: Review Needed 👀 Added to PRs when they need more review label Jun 15, 2021
@Tbaut Tbaut enabled auto-merge (squash) June 15, 2021 15:54
@Tbaut Tbaut merged commit f1852e9 into dev Jun 15, 2021
@Tbaut Tbaut deleted the feat/import-pin-1105 branch June 15, 2021 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a pin (import by CID) Modal
4 participants