Conversation
There was a problem hiding this comment.
Pull request overview
Implements a persisted filesystem domain model (hierarchical directories + files) with logical path generation, Active Storage-backed file content, and supporting RSpec specs plus a demo Rake task.
Changes:
- Added
Directory(recursive hierarchy + cycle prevention) andFileEntry(directory-scoped uniqueness + Active Storage attachment) models. - Added RSpec model specs for validations/associations/path generation and Active Storage attachment behavior.
- Added project scaffolding for Docker-based dev, Rails configuration (storage, locales), and CI/lint/security tooling.
Reviewed changes
Copilot reviewed 76 out of 101 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/.keep | Keeps vendor dir in git |
| tmp/pids/.keep | Keeps tmp/pids dir in git |
| tmp/.keep | Keeps tmp dir in git |
| spec/spec_helper.rb | RSpec base config |
| spec/rails_helper.rb | Rails + RSpec integration/config |
| spec/models/file_entry_spec.rb | FileEntry model specs |
| spec/models/directory_spec.rb | Directory model specs |
| script/.keep | Keeps script dir in git |
| public/robots.txt | robots.txt placeholder |
| public/icon.svg | App icon (SVG) |
| public/icon.png | App icon (PNG) |
| public/500.html | Static 500 page |
| public/422.html | Static 422 page |
| public/406-unsupported-browser.html | Static unsupported browser page |
| public/404.html | Static 404 page |
| public/400.html | Static 400 page |
| log/.keep | Keeps log dir in git |
| lib/tasks/filesystem_demo.rake | Demo task for filesystem behavior |
| lib/tasks/.keep | Keeps lib/tasks dir in git |
| docker-compose.yml | Dev Docker composition (web + postgres) |
| db/seeds.rb | Seeds placeholder |
| db/schema.rb | Schema including directories/file_entries + Active Storage |
| db/queue_schema.rb | Solid Queue schema |
| db/migrate/20260413204100_create_file_entries.rb | Migration for file_entries |
| db/migrate/20260413204058_create_directories.rb | Migration for directories |
| db/migrate/20260413203158_create_active_storage_tables.active_storage.rb | Active Storage migrations |
| db/cache_schema.rb | Solid Cache schema |
| db/cable_schema.rb | Solid Cable schema |
| config/storage.yml | Active Storage disk services (local/test) |
| config/routes.rb | Routes skeleton + healthcheck |
| config/recurring.yml | Solid Queue recurring config |
| config/queue.yml | Solid Queue config |
| config/puma.rb | Puma config |
| config/locales/pt-BR.yml | pt-BR AR model + error translations |
| config/locales/en.yml | en locale placeholder |
| config/initializers/inflections.rb | Inflections placeholder |
| config/initializers/filter_parameter_logging.rb | Parameter filtering config |
| config/initializers/content_security_policy.rb | CSP placeholder |
| config/initializers/assets.rb | Assets config |
| config/environments/test.rb | Test env config (Active Storage test) |
| config/environments/production.rb | Production env config |
| config/environments/development.rb | Development env config (Active Storage local) |
| config/environment.rb | Rails environment boot |
| config/deploy.yml | Kamal deploy config |
| config/database.yml | Postgres DB config via env |
| config/credentials.yml.enc | Encrypted credentials |
| config/ci.rb | Local CI script definition |
| config/cache.yml | Solid Cache config |
| config/cable.yml | Action Cable config |
| config/bundler-audit.yml | bundler-audit config |
| config/boot.rb | Bundler/bootsnap boot |
| config/application.rb | Rails app config + i18n defaults |
| config.ru | Rack entrypoint |
| bin/thrust | Thruster launcher |
| bin/setup | Dev setup script |
| bin/rubocop | RuboCop launcher |
| bin/rake | Rake launcher |
| bin/rails | Rails launcher |
| bin/kamal | Kamal launcher |
| bin/jobs | Solid Queue runner |
| bin/docker-entrypoint | Container entrypoint (db:prepare) |
| bin/dev | Dev server script |
| bin/ci | Local CI runner |
| bin/bundler-audit | bundler-audit launcher |
| bin/brakeman | Brakeman launcher |
| app/views/pwa/service-worker.js | PWA service worker template |
| app/views/pwa/manifest.json.erb | PWA manifest template |
| app/views/layouts/application.html.erb | Base HTML layout |
| app/models/file_entry.rb | FileEntry model |
| app/models/directory.rb | Directory model |
| app/models/concerns/.keep | Keeps concerns dir in git |
| app/models/application_record.rb | ApplicationRecord base |
| app/helpers/application_helper.rb | Helper skeleton |
| app/controllers/concerns/.keep | Keeps controller concerns dir in git |
| app/controllers/application_controller.rb | Base controller config |
| app/assets/stylesheets/application.css | Stylesheet manifest |
| app/assets/images/.keep | Keeps images dir in git |
| Rakefile | Loads Rails rake tasks |
| README.md | Project documentation |
| Gemfile.lock | Dependency lockfile |
| Gemfile | Gems (Rails, RSpec, Shoulda, etc.) |
| Dockerfile.dev | Dev Docker image |
| Dockerfile | Production Docker image |
| .ruby-version | Ruby version pin |
| .rubocop.yml | RuboCop config |
| .rspec | RSpec options |
| .kamal/secrets | Kamal secrets sourcing |
| .kamal/hooks/pre-proxy-reboot.sample | Kamal hook sample |
| .kamal/hooks/pre-deploy.sample | Kamal hook sample |
| .kamal/hooks/pre-connect.sample | Kamal hook sample |
| .kamal/hooks/pre-build.sample | Kamal hook sample |
| .kamal/hooks/pre-app-boot.sample | Kamal hook sample |
| .kamal/hooks/post-proxy-reboot.sample | Kamal hook sample |
| .kamal/hooks/post-deploy.sample | Kamal hook sample |
| .kamal/hooks/post-app-boot.sample | Kamal hook sample |
| .kamal/hooks/docker-setup.sample | Kamal hook sample |
| .gitignore | Git ignores (logs/tmp/storage/keys) |
| .github/workflows/ci.yml | GitHub Actions CI (security + lint) |
| .github/dependabot.yml | Dependabot config |
| .gitattributes | Linguist + credentials diff rules |
| .dockerignore | Docker build context ignores |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: bin/brakeman --no-pager | ||
|
|
||
| - name: Scan for known security vulnerabilities in gems used | ||
| run: bin/bundler-audit |
There was a problem hiding this comment.
The CI workflow currently runs Brakeman/Bundler Audit and RuboCop, but it does not run the RSpec suite. Given this PR adds model logic + validations + ActiveStorage behavior, add a test job that provisions Postgres and runs bundle exec rspec so regressions are caught in CI.
| run: bin/bundler-audit | |
| run: bin/bundler-audit | |
| test: | |
| runs-on: ubuntu-latest | |
| services: | |
| postgres: | |
| image: postgres:16 | |
| env: | |
| POSTGRES_USER: postgres | |
| POSTGRES_PASSWORD: postgres | |
| POSTGRES_DB: app_test | |
| ports: | |
| - 5432:5432 | |
| options: >- | |
| --health-cmd="pg_isready -U postgres -d app_test" | |
| --health-interval=10s | |
| --health-timeout=5s | |
| --health-retries=5 | |
| env: | |
| RAILS_ENV: test | |
| PGHOST: localhost | |
| PGUSER: postgres | |
| PGPASSWORD: postgres | |
| PGDATABASE: app_test | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v6 | |
| - name: Set up Ruby | |
| uses: ruby/setup-ruby@v1 | |
| with: | |
| bundler-cache: true | |
| - name: Prepare test database | |
| run: bin/rails db:prepare | |
| - name: Run RSpec | |
| run: bundle exec rspec |
| ## Quick Start | ||
|
|
||
| ```bash | ||
| git clone git@github.com:flaviolpgjr/ruby-dev-test-1.git |
There was a problem hiding this comment.
The README hard-codes a personal repository URL in the Quick Start (git@github.com:flaviolpgjr/ruby-dev-test-1.git). This will be incorrect for other forks/CI checks. Prefer using a placeholder or instructing the reader to clone the current repository instead of a specific user/org URL.
| git clone git@github.com:flaviolpgjr/ruby-dev-test-1.git | |
| git clone <repository-url> |
| Shoulda::Matchers.configure do |config| | ||
| config.integrate do |with| | ||
| with.test_framework :rspec | ||
| with.library :rails | ||
| end | ||
| end |
There was a problem hiding this comment.
The block variable in Shoulda::Matchers.configure do |config| shadows the outer RSpec.configure do |config|, which makes the configuration harder to read and easy to misuse. Use a different block variable name for the Shoulda config to avoid confusion.
| config.before(:each) do | ||
| ActiveStorage::Attachment.delete_all | ||
| ActiveStorage::Blob.delete_all | ||
| FileEntry.delete_all | ||
| Directory.delete_all | ||
| FileUtils.rm_rf(Rails.root.join("storage")) | ||
| FileUtils.rm_rf(Rails.root.join("tmp/storage")) | ||
| FileUtils.mkdir_p(Rails.root.join("storage")) | ||
| FileUtils.mkdir_p(Rails.root.join("tmp/storage")) |
There was a problem hiding this comment.
This before(:each) hook truncates multiple tables and recursively deletes/recreates the storage directories for every example, even though use_transactional_fixtures = true is enabled. This will significantly slow the suite and can cause flakiness if specs ever run in parallel. Consider scoping the cleanup to only specs that attach files (e.g., via metadata) and/or using a lighter cleanup strategy (purge only created blobs / clear the test storage directory in an after(:each) hook).
|
|
||
| add_index :directories, [:parent_id, :name], unique: true | ||
| end |
There was a problem hiding this comment.
This migration adds a unique index on [:parent_id, :name], but because parent_id is nullable Postgres will allow multiple rows with parent_id = NULL and the same name (NULLs are distinct). That means duplicate root directories can exist at the DB level under concurrency or when validations are bypassed. Add a DB constraint that also enforces uniqueness for root directories (e.g., a partial unique index on name where parent_id IS NULL, or use a nulls_not_distinct index option if supported).
| if ActiveStorage::Blob.service.is_a?(ActiveStorage::Service::DiskService) | ||
| report_storage_path = ActiveStorage::Blob.service.send(:path_for, report.content.blob.key) | ||
| invoice_storage_path = ActiveStorage::Blob.service.send(:path_for, invoice.content.blob.key) |
There was a problem hiding this comment.
This uses ActiveStorage::Blob.service.send(:path_for, ...), which relies on a private API and will not work for non-disk services. Prefer using the public service API (e.g., ActiveStorage::Blob.service.exist?(key)), and only print a filesystem path when the adapter explicitly supports it.
| require "rails_helper" | ||
|
|
There was a problem hiding this comment.
StringIO is used later in this spec but the file never requires the stdlib "stringio". In a clean Ruby process this can raise NameError: uninitialized constant StringIO. Add require "stringio" (e.g., at the top of this spec or in rails_helper).
| config.before(:each) do | ||
| ActiveStorage::Attachment.delete_all |
There was a problem hiding this comment.
There is an extra leading space before config.before(:each) which will likely trigger RuboCop indentation/layout cops. Fix the indentation so it aligns with the surrounding RSpec config block.
| validates :name, presence: true, uniqueness: { scope: :directory_id } | ||
| validates :directory, presence: true |
There was a problem hiding this comment.
FileEntry#path joins directory.path and name with "/", but there is no validation preventing name from containing "/". That can generate incorrect/ambiguous logical paths. Add a format validation on name to disallow path separators (and keep it consistent with Directory).
| begin | ||
| puts "Limpando dados anteriores..." | ||
| ActiveStorage::Attachment.delete_all | ||
| ActiveStorage::Blob.delete_all | ||
| FileEntry.delete_all | ||
| Directory.delete_all | ||
| cleanup_storage! |
There was a problem hiding this comment.
filesystem:demo deletes all ActiveStorage records, all Directory/FileEntry records, and wipes storage/ + tmp/storage/ without any environment guard. If someone runs this task in the wrong environment it will cause data loss. Add a safety check (e.g., abort unless Rails.env.development? / test?) and/or require an explicit confirmation flag.
Importante
Caso precise isolar o pr em uma release separada, favor incluir no titulo do PR a informação [rls_isolada]
ex: [ARCH-123][rls_isolada]Atualização de Excalibur
Descrição
Este PR implementa a modelagem de um sistema de arquivos com suporte a diretórios hierárquicos e arquivos.
Principais pontos:
A solução prioriza consistência de domínio e separação entre estrutura lógica e armazenamento físico.
Issue tracker
[Link para o card no Jira.]
Code Review
Como fazer o Code Review:
CONTRIBUTING.md
Screenshots (para mudanças de UI, se houver)
[Adicione alguns screenshots das mudanças de UI inclusas no seu PR, se houver. Isso ajuda o revisor a entender como o usuário utilizaria o código do PR.]
Links e observações
[Links úteis que podem contextualizar e ajudar o revisor, por exemplo para a página de uma dependência que escolheu adicionar, ou um código que se inspirou, ou documentação externa (docs de uma API, do Vue, do Rails, etc).]
Checklist para poder mergear