seekr-osint / seekr

A multi-purpose OSINT toolkit with a neat web-interface.
GNU General Public License v3.0
515 stars 43 forks source link

[RFC] Code quality guidelines for the website and general concerns #550

Closed 9glenda closed 11 months ago

9glenda commented 1 year ago

Spending the last 4 hours fixing and discovering new bugs in seekr as part of #497 I came to the conclusion that the website is full of bugs and unscalable code. Compared to the api which is testet for bugs via unit tests the website has no such code quality control mechanism. The core cause for many of those bugs is that the website does not use a UI framework which is totally fine and reasonable. But without a UI framework over the time the code often suffers due to missing features like templating ... Personally I don't think seekr has to use a UI framework keeping in mind the big effort associated with switching to a UI Framework. Still I think html UI templating for example should still be done. This can easily be done through the go standard libary for html parsing and templating.

Code duplication

A big issue of the website code it the enormous amount of code duplication. Examples:

Drop Down creation

Very similar code is use for the dropdown creation. Gender dropdown implementation: https://github.com/seekr-osint/seekr/blob/82d7f223d4bf81adacf585a704459685c53c79d5/web/ts/script.ts#L32-L45 Ethincity dropdown implementation: https://github.com/seekr-osint/seekr/blob/82d7f223d4bf81adacf585a704459685c53c79d5/web/ts/script.ts#L47-L61 This issue I started addressing in #497

My solution to the issue

Those tasks only differ in the drop down name, variable to store the value in and nth-child position (this could potentially be addressed in the future to automatically detect the nth-child) and therefore should always be implemented through a function with scalability in mind. https://github.com/seekr-osint/seekr/blob/34c1159e3f56f440cfec86907e0ffb1b75fe1920/web/ts/script.ts#L4-L18 For the actual implementation of the dropdown we now have a 1-liner making it not only easier to read and easier to update also really easy to scale by adding new dropdowns (The dropdown code has other issues which would also need to be addressed with the same goal in mind). https://github.com/seekr-osint/seekr/blob/34c1159e3f56f440cfec86907e0ffb1b75fe1920/web/ts/script.ts#L134 https://github.com/seekr-osint/seekr/blob/34c1159e3f56f440cfec86907e0ffb1b75fe1920/web/ts/script.ts#L136

How does this approach prevent bugs/help fix them faster?

If there's a bug found like #552 it can be tricky to search all the duplicated code for rendering those items. Having to change only one implementation of the rendering makes it a easy and fast task to fix this bug.

How to combat this issue?

Following parts of the website should be rewritten because of being the root cause of a lot of bugs:

  1. Translation (see #548 #549).
  2. Dropdowns they cause a lot of issue and have a lot of code duplication.

We need to stop adding new features and fix the core features of seekr right now

Conclusion

The issue is not that Nite or me with (#485 a PR which removed a lot of duplicated code but also introduced a lot of bugs) are just bad at coding. The issue is that the website code is not always written with scalability in mind and now this is causing a lot of issues.

Related Issues

549

548

497

552

9glenda commented 1 year ago

@Niteletsplay

cn1t commented 12 months ago

Fixed in #551