search

WildAid / o-fish-web

Web application for the Officer's Fishery Information Sharing Hub (O-FISH). The web app allows agencies to gain insights from the aggregated information gathered during a routine vessel inspection (submitted via the web app).
Apache License 2.0
31 stars 41 forks source link

Replace JSON queries in the URL with query params application wide #343

Open sourabhbagrecha opened 3 years ago

sourabhbagrecha commented 3 years ago

Is your feature request related to a problem? Please describe. So the current URLs looks something like this: /vessels/view/{ vessel.permitNumber: 12345 ,vessel.name: MyBoat} If you can see that we are using JSON-based query in the above URL wrapped in curly braces, well this might make the initial navigation work pretty easy but this may cause errors and other navigation issues when we will be scaling the app with more features.

Describe the solution you'd like The solution to this issue is to use standard query params, so for the above example, the URL would now look like: /vessels/view?permitNumber=12345&name=MyBoat this will drastically reduce the chances of getting unexpected errors due to ill-formatted JSON queries in the URL, also this approach is highly extensible and scales well with new features and complex navigation stuff.

Sheeri commented 3 years ago

Hi @sourabhbagrecha - great issue, this is ready to be worked on whenever someone has time. Would you like it assigned to you?

Sheeri commented 3 years ago

@sourabhbagrecha would this also get rid of the # in the URL? e.g. https://hostname.com/#/vessels/view/......

ayushjainrksh commented 3 years ago

@sourabhbagrecha would this also get rid of the # in the URL? e.g. https://hostname.com/#/vessels/view/......

@Sheeri It is a different issue. # in the URL is because of the use of Hash Router in React. Generally, it's better to use Browser Router in which there is no # in the URL.

https://github.com/WildAid/o-fish-web/blob/c27da4bde3d58ad3284620c73cbcc76080f382fc/src/root/root.history.js#L1-L4

Sheeri commented 3 years ago

Ah, OK. I've done some reading on that and it does seem like it would be good to do. Do you know how much would need to change in the code to make that happen? I'd hate to have unintended consequences, and if it's a large change we might just leave well enough alone.

sourabhbagrecha commented 3 years ago

Yes, the # will be removed thereafter. Also I can start working on this, please assign it to me. I am not sure about the estimated time but I can start at least.

Sheeri commented 3 years ago

OK, no worries. Just keep in touch and let us know! :D

ayushjainrksh commented 3 years ago

Ah, OK. I've done some reading on that and it does seem like it would be good to do. Do you know how much would need to change in the code to make that happen? I'd hate to have unintended consequences, and if it's a large change we might just leave well enough alone.

Replacing it right now might have consequences. I tried that. Solving this issue should solve the whole problem so I'm rooting for @sourabhbagrecha :)

Sheeri commented 2 years ago

@sourabhbagrecha - is this something you might pick up for this year's Hacktoberfest? Totally OK if you can't prioritize it, I'll unassign it if that's the case.

sourabhbagrecha commented 2 years ago

Hi Sheeri, seems like I won't be able to prioritize this in the near future. Apologies for the inconvenience caused.