server icon indicating copy to clipboard operation
server copied to clipboard

Document database projects and complete EDD support

Open withinfocus opened this issue 7 months ago â€ĸ 3 comments

đŸŽŸī¸ Tracking

https://bitwarden.slack.com/archives/C024Z7LQNLB/p1747902781176209

📔 Objective

Adds some light documentation within the various SQL projects to explain their purpose and how they inter-relate, with links out to usage.

Completes what's needed for online environments to use EDD, specifically non-backwards-compatible migrations.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

đŸĻŽ Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or â„šī¸ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or âš ī¸ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or â™ģī¸ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

withinfocus avatar May 22 '25 18:05 withinfocus

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.65%. Comparing base (9a501f9) to head (e310ef7). Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5855      +/-   ##
==========================================
+ Coverage   51.46%   51.65%   +0.18%     
==========================================
  Files        1729     1731       +2     
  Lines       76373    76430      +57     
  Branches     6820     6824       +4     
==========================================
+ Hits        39305    39477     +172     
+ Misses      35526    35400     -126     
- Partials     1542     1553      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar May 22 '25 18:05 codecov[bot]

Logo Checkmarx One – Scan Summary & Details – 31490518-73f2-419d-b20f-973879e7510e

Great job, no security vulnerabilities found in this Pull Request

github-actions[bot] avatar May 22 '25 18:05 github-actions[bot]

We have a couple of more files that needs to be moved. https://github.com/bitwarden/server/tree/main/src/Sql

Might be useful to add a linter step which errors if teams adds files outside any of the dbo folders? Beyond that I think the date rename is going to cause issues and be something we have to address before this can be run automatically.

I fixed this in #6094, not sure why that was done when it's visible that nothing else is set up that way. Linting this feels like overkill -- you need to put certain files in certain places ...

They would be run on the next release when they are prompted from finalization to regular migrations?

This was another mistake that just hadn't been noticed since we haven't had a transition migration since. There's no process change and you can trace the constant usage but the transition ones run automatically upon deployment after the regular migrations.

It was but we need to ensure migrations are run in the correct order. If we have a migration named YYYY-0M-FinalizationMigration.sql as per the docs, this will probably run out of order as, and it should be prefixed with the full date of the PR. I.e. if we open the EDD PR on 2025-07-05, the date prefix needs to change to 2025-07-05_00_FinalizationMigration.

I can add this.

withinfocus avatar Jul 17 '25 13:07 withinfocus

@Hinton this is ready again -- I don't think there's anything else to do.

withinfocus avatar Jul 18 '25 20:07 withinfocus