Skip to content

fix: Adjust relationship between ServerGroup and Server/SharedServer#10076

Closed
lkmatsumura wants to merge 1 commit into
pgadmin-org:masterfrom
lkmatsumura:fix_relation
Closed

fix: Adjust relationship between ServerGroup and Server/SharedServer#10076
lkmatsumura wants to merge 1 commit into
pgadmin-org:masterfrom
lkmatsumura:fix_relation

Conversation

@lkmatsumura

@lkmatsumura lkmatsumura commented Jun 10, 2026

Copy link
Copy Markdown
  • Adjusting relationship

    • ServerGroup has many Servers
    • ServerGroup has many SharedServers
    • Server has one ServerGroup not servers
    • SharedServer has one ServerGroup not servers
    • Using back_populates instead of backref for better control
  • Adjusted references to ServerGroup in server (was server.serves): server.servergroup.name instead server.servers.name

  • Not find references to ServerGroup in SharedServer (sharedServer.servers)

Summary by CodeRabbit

  • Refactor
    • Restructured database model relationships to improve server organization and grouping management.
    • Updated server grouping references in schema comparison and SQL editor tools for consistency across the application.

- Adjusting relationship
  - ServerGroup has many Servers
  - ServerGroup has many SharedServers
  - Server has one ServerGroup not servers
  - SharedServer has one ServerGroup not servers
  - Using back_populates instead of backref for better control

- Adjusted references to ServerGroup in server (was server.serves):
  server.servergroup.name instead server.servers.name
- Not find references to ServerGroup in SharedServer (sharedServer.servers)
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98abe1cb-9bea-431c-8236-883f385dd9a5

📥 Commits

Reviewing files that changed from the base of the PR and between 04fa05c and e94c5ec.

📒 Files selected for processing (3)
  • web/pgadmin/model/__init__.py
  • web/pgadmin/tools/schema_diff/__init__.py
  • web/pgadmin/tools/sqleditor/__init__.py
 ________________________________________
< I'm a lean, mean, code review machine. >
 ----------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

Flake8 can be used to improve the quality of Python code reviews.

Flake8 is a Python linter that wraps PyFlakes, pycodestyle and Ned Batchelder's McCabe script.

To configure Flake8, add a '.flake8' or 'setup.cfg' file to your project root.

See Flake8 Documentation for more details.

@dpage dpage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a reasonable readability cleanup — the Server.servers relationship was confusingly named (it pointed to the one ServerGroup), so Server.servergroup / ServerGroup.servers + back_populates reads much better.

I checked the rest of the tree: the three call sites you updated (schema_diff ×3, sqleditor ×1) are the only real users of the old server.servers attribute, and nothing references the old auto-generated backref names (servergroup.server / .sharedserver), so dropping them is safe. The cascade is correctly carried over onto the new ServerGroup.servers/sharedservers relationships, and the FK columns are unchanged — so no schema/migration impact.

Two things before merge:

  • No changelog entry — this still wants a one-line "Housekeeping" entry in the current docs/en_US/release_notes_*.rst.
  • It's a pure rename with no functional change and no test; that's fine for a cleanup, but worth confirming the relationship rename doesn't disturb any serialization that relied on the attribute name.

dpage pushed a commit that referenced this pull request Jun 19, 2026
#10076)

The Server and SharedServer models each declared a relationship named
'servers' that actually pointed at the parent ServerGroup, with backrefs
('server'/'sharedserver') providing the collections. The naming was the
inverse of what it modelled and made the call sites read oddly.

Redefine the relationships in the natural direction using explicit
back_populates: ServerGroup gains 'servers' and 'sharedservers'
collections (carrying the existing delete-orphan cascade), and Server and
SharedServer each gain a 'servergroup' reference. The two call sites that
read the group name are updated from server.servers.name to
server.servergroup.name accordingly.

This is a naming/modelling cleanup with no functional or schema change, so
no migration is required.
@dpage

dpage commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Merged to master as dfc4ef3 (squashed; the SHA differs so GitHub won't auto-close this). I folded in a small tidy on top of your change, dropping the cosmetic blank lines between the existing column definitions so the diff stays purely semantic, and reworded the subject to refactor: since this is a naming/modelling cleanup with no behavioural change. I confirmed the rewired back_populates mappers configure cleanly at app startup and verified the two call sites were the only consumers of the old relationship. Thanks for untangling this, the new direction reads far better!

@dpage dpage closed this Jun 19, 2026
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