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

Feature Addition: Account Linker/Switcher && Bugfix: Enable staff to see hidden threads #209

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

LegendBegins
Copy link
Collaborator

Hiding a thread is no longer permanent, preventing 1-click errors (as appears to be the original intent).

Remaining TODO: Threads still show up on the homepage in the Latest section after deletion. If mods wipe a thread, that should trigger an update.

Missing return statement in cancan.js, rendering deleted threads gone forever
Added SQL query for staff to retrieve hidden topics. Had the bonus effect of coalescing two different queries that otherwise had to be synchronized, now managed by one function.
Added check to see if user was staff, and if so, expose hidden threads on /forums/:forumSlug
@LegendBegins LegendBegins changed the title Bugfix: Enable staff to see hidden threads Feature Addition: Account Linker/Switcher && Bugfix: Enable staff to see hidden threads Jun 12, 2023
@LegendBegins
Copy link
Collaborator Author

LegendBegins commented Jun 12, 2023

Major caveat: This commit requires an addition to the DB schema. Specifically, a two-column table for account alts and pre-populating that table with all user accounts. The two queries I added to schema and dev_seeds should get everything set up. Reproduced below for clarity:

CREATE TABLE alts (
id SERIAL PRIMARY KEY,
ownerId SERIAL NOT NULL
);

INSERT INTO alts (id, ownerId)
SELECT id, id
FROM users;

Additional notes: I did some onclick JS in macros.html to support the dropdown menu, and left it inline since it was pretty much a one-liner. If you want me to break it into its own function in the JS file, let me know and I'll modify it.

There's a lot of code to verify here, so feel free to ping me on Discord with any questions—I think most of it should be pretty straightforward.

I do need to double check that the alts table is updated on registration. The query was super simple and seemed to work when I pasted it into Postgres, but since I haven't messed around with the test environment to get CAPTCHAs working (or bypass the requirement) I wasn't able to verify that one piece of functionality within the environment context.

@danneu
Copy link
Owner

danneu commented Jan 3, 2025

  1. The alts table contains pointers into the users table, so we can make them foreign keys with REFERENCES.
  2. Postgres has case insensitive names so the convention is to use snake case to avoid confusion: ownerId -> owner_id
  3. You do it in dev_seeds.sql but we should move the original seeding of the alts table into the same migration that creates the table (aka next to the create table in schema.sql).

I think all of that put together is this:

CREATE TABLE alts (
  id         integer     PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE,
  owner_id   integer     NOT NULL REFERENCES users(id) ON DELETE CASCADE,
  created_at timestamptz  NOT NULL DEFAULT NOW() -- useful for these bridge tables, esp debugging
);

-- There's already an index on primary keys but lets add one for the foreign key.
CREATE INDEX idx_alts_owner_id ON alts(owner_id);

-- Initialize with existing users
INSERT INTO alts (id, owner_id)
SELECT id, id
FROM users;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants