neulab / explainaboard_client

1 stars 1 forks source link

feat: now displays a link to view the system just submitted #22

Closed PaulCCCCCCH closed 2 years ago

PaulCCCCCCH commented 2 years ago

Related issue here Will require this PR to be merged at the frontend for the link to work.

lyuyangh commented 2 years ago

Thanks for the PR!

I just have one suggestion about how the frontend URL is determined. Instead of hardcoding the frontend URL/route (e.g. {frontend}/systems?system_id={sys_id}) in this repo, I think it'll increase maintainability if the POST /systems endpoint returns the URL. This is more maintainable because if we ever decide to change the frontend route for the systems page, we don't need to update this repo simultaneously.

PaulCCCCCCH commented 2 years ago

As discuss in the previous meeting, we will change the backend to return the complete url in another PR. I've created another issue for this. Check here.