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

Add loading and error indications in the database page #3330

Closed
pavish opened this issue Nov 30, 2023 · 9 comments · Fixed by #3351
Closed

Add loading and error indications in the database page #3330

pavish opened this issue Nov 30, 2023 · 9 comments · Fixed by #3351
Labels
help wanted Community contributors can implement this ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@pavish
Copy link
Member

pavish commented Nov 30, 2023

Description

  • The database page does not show any loading or error indicators when fetching the list of schemas.
  • So far, the common_data loaded the schemas that were initially displayed in the database page, and hence the issue was not immediately visible in the UI.
  • With the new Connections page, users can add a new connection and update existing ones. This leads to having to fetch the list of schemas while the page is open. This results in the page showing Schemas (0) and nothing else, while the schemas are being fetched or if an error has occurred while fetching.
Screenshot 2023-12-01 at 3 25 26 AM

Expected behavior

  • The database page should show a loading indicator (a skeleton), while it's schemas are being fetched.
  • Show error messages in the database page if the schema list fetch request has failed.
@pavish pavish added type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory ready Ready for implementation labels Nov 30, 2023
@pavish pavish added this to the v0.1.4 milestone Nov 30, 2023
@pavish
Copy link
Member Author

pavish commented Nov 30, 2023

I'm placing this in the 0.1.4 milestone since I think most users will start facing this issue after the Database Connections UI goes out.

@MaYaNkKashyap681
Copy link

I want to work on this issue, could you please assign this issue to me?

@pavish
Copy link
Member Author

pavish commented Dec 4, 2023

@MaYaNkKashyap681 You're welcome to work on this and submit a PR, but as noted in our Contributor Guide, we don't assign tickets to new contributors.

Once we've already merged a PR of yours, then we'll be happy to assign tickets to you after that.

Also note that this issue might be prioritized for our upcoming release. So, if we don't get a PR from you when we're close to the release, one of the maintainers might pick it up.

@MaYaNkKashyap681
Copy link

Sure

@cerdo03
Copy link

cerdo03 commented Dec 5, 2023

Can anyone help in understanding in how the schemas are being fetched from the db? I dont see any other api being hit other than users/ in the network tab.

@MaYaNkKashyap681
Copy link

Can anyone help in understanding in how the schemas are being fetched from the db? I dont see any other api being hit other than users/ in the network tab.

Right, i have also noticed same.

@seancolsen seancolsen added the help wanted Community contributors can implement this label Dec 6, 2023
@pavish
Copy link
Member Author

pavish commented Dec 6, 2023

@cerdo03 @MaYaNkKashyap681

There is a property called common_data which is rendered in the Django templates when the app loads for the first time. It already contains the list of schemas, and the frontend does not need to make an API request to fetch schemas separately. I'd recommend you to look at the code in mathesar/templates/mathesar/index.html, mathesar/mathesar/views.py, and mathesar_ui/src/stores/schemas.ts.

The issue above presents itself only when there are multiple databases connected to Mathesar, as noted in the description. There's current ongoing work to allow connecting to new databases from the UI.

You can connect to more databases manually by following these steps:

  • Edit docker-compose.dev.yml and modify this line,
    from
    MATHESAR_DATABASES=(mathesar_tables|postgresql://mathesar:mathesar@mathesar_dev_db:5432/mathesar) 
    
    to
    MATHESAR_DATABASES=(mathesar_tables|postgresql://mathesar:mathesar@mathesar_dev_db:5432/mathesar),(duplicate_mathesar|postgresql://mathesar:mathesar@mathesar_dev_db:5432/mathesar),(second_db|postgresql://mathesar:mathesar@mathesar_dev_db:5432/newdb)
    
  • Do not commit this change, only make it locally.
  • Restart Mathesar.
  • You should now see three database connections in the UI. Click on one of them, and you'll notice that the frontend sends a request to the api/db/v0/schemas endpoint.

@nikhilhenry
Copy link
Contributor

@pavish

Currently the schema store uses the deprecated States enum. As part of this issue, would that have to be refactored to use RequestStatus instead?

@pavish
Copy link
Member Author

pavish commented Dec 12, 2023

Currently the schema store uses the deprecated States enum. As part of this issue, would that have to be refactored to use RequestStatus instead?

@nikhilhenry Yes, that would be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Community contributors can implement this ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants