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

fix(account-block): evaluate implementation #117

Open
dargmuesli opened this issue Jan 17, 2025 · 0 comments
Open

fix(account-block): evaluate implementation #117

dargmuesli opened this issue Jan 17, 2025 · 0 comments
Assignees

Comments

@dargmuesli
Copy link
Member

dargmuesli commented Jan 17, 2025

Following #73 (and cleanup PR #116), some questions came up in the final review:

  1. function_events_invited:

    1. shouldn't blocking be based on the event author instead of the contact author?
  2. function_invitation_claim_array:

    1. how could _invitation_ids be null?
    2. where is append_invitation_array(result_invitation_ids, _invitation_id) defined? ✅
    3. is the function stable still?
  3. function_invitation_contact_ids:

    1. is the NULL check necessary for blocking? (currently line 24)
    2. is the blocking check even necessary here as invitation_claim_array and events_organized already account for blocking?
  4. table_account_block:

    1. can you block yourself?
    2. why did I comment to remove the delete mutation on the table?
    3. i'd say the select policy should only allow entries that were created by the invoker, not the ones that target the invoker. It's common in social networks to not definitely know "who blocked me" but only see no data (e.g. empty profiles) of people who blocked the invoker.
  5. table_account_block_policy:

    1. why are selections allowed for anonymous users?
    2. is maevsi.invoker_account_id() IS NOT NULL necessary here and in other files?
  6. table_event_category_mapping_policy:

    1. does the selection based on visibility make sense?
  7. table_invitation_policy:

    1. does the select filter for blocking on own events make sense?
  8. sqitch.plan:

    1. could the tests be placed in a way that does not require a test schema? (more of a challenge for myself)
  9. all:

    1. should we extract the common UNION selection on the account_block table into a function? ✅

Let's talk about these questions in person @sthelemann

@dargmuesli dargmuesli added the bug Something isn't working label Jan 17, 2025
@dargmuesli dargmuesli moved this to 🔖 Ready in maevsi Jan 17, 2025
@dargmuesli dargmuesli removed the bug Something isn't working label Jan 17, 2025
@dargmuesli dargmuesli moved this from 🔖 Ready to 🏗 In progress in maevsi Jan 25, 2025
@dargmuesli dargmuesli removed their assignment Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

No branches or pull requests

2 participants