Skip to content
This repository has been archived by the owner on Dec 2, 2022. It is now read-only.

[WIP] Yet Another Over-Sized PR [Featured, add business, edit business, set location] #34

Merged
merged 6 commits into from
Feb 28, 2021

Conversation

microfauna
Copy link
Collaborator

@microfauna microfauna commented Feb 20, 2021

This PR has rolled in quite a number of bug fixes and re-factors that ended up affecting the core feature-set it's supposed to touch. Hopefully once we reach v1, we can slow down the PRs to more iterative changes and test thoroughly.

This PR introduces a good number of features and bug fixes:

  • Adding businesses to your account
  • Editing businesses
    • Can set all descriptive info, territories, tags
    • Can set geolocation
    • Basic user feedback when saving
  • Admins can now mark businesses as featured and they will show up on the front page
  • [FIX] Updated Vue-Select for new events
  • Simplify locations to be embedded in the business.
  • Move Map to its own component for re-usability.

And introduces some tech-debt (checkmarks to create tickets):

  • Geocoding
    • It can behave a bit funkily. It's not a proper multi-result search but does its best to approximate the latitude and longitude
    • We should get off of google entirely, but for now their geocoding service works. [DevOps] Get off google geocoding #35
  • User images
    • These need a lot more consideration. We should shoot to get proper storage working even if it's temporary and only on our server. URLs introduce a lot of potential for abuse or bugs, and introduce more tech debt later on. [DevOps] Create storage for owner images #36
  • Database
  • Finish deletion. The functionality is there but not the hasura mutation. [FE] Delete business #13
  • Move to a better querying library. Apollo has some issues. [FE] Move to using urql for graphql usage #37
  • serious / security Hasura has an issue where a constraint cannot depend on a newly inserted relationship. Basically you can enforce that new business owners only belong to the logged in user, and for newly created businesses - but you can't enforce that a business creation has an owner. This would allow a user to create zombie businesses with no owners that only admins can delete or update. It would not allow for a user to escalate privileges but could allow anonymous flooding of the database. Possible solutions:
    • simplest: Add a "created_by" field, separate from owners / editors [BE] Add created_by field for each new business #38
    • Run a garbage collection job on rogue businesses
    • In order to do this you'd have to be a malicious actor / modify the query, and we can just ban people who do that.

Images:
image

@microfauna microfauna requested a review from arecvlohe February 20, 2021 23:44
This was linked to issues Feb 28, 2021
@arecvlohe arecvlohe added the v1 label Feb 28, 2021
Copy link
Contributor

@arecvlohe arecvlohe left a comment

Choose a reason for hiding this comment

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

Overall looks good to me!

@arecvlohe
Copy link
Contributor

Added issues for each item in the description of the PR so we can address in v2

@microfauna microfauna merged commit caa8619 into main Feb 28, 2021
@microfauna microfauna deleted the ns/modify-business-view branch February 28, 2021 20:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] Preview business [FE] Create location [FE] Create business
3 participants