Skip to content

Commit

Permalink
Merge pull request #312 from stumpapp/al/0.0.2-fixes
Browse files Browse the repository at this point in the history
0.0.2 bug hunting (round 2)
  • Loading branch information
aaronleopold committed Apr 20, 2024
2 parents 724f7e0 + f82ee09 commit 4beb7b3
Show file tree
Hide file tree
Showing 13 changed files with 131 additions and 119 deletions.
57 changes: 14 additions & 43 deletions .github/CONTRIBUTING.md
Expand Up @@ -6,65 +6,36 @@ To ensure nobody's time and effort is wasted, please be sure to follow the guide

1. Check to see if an issue already exists relevant to your feature/topic
2. Create an issue (if an issue does not already exist) and express interest in working it (see the [issues](#issues) section below)
3. Create a new feature branch **off of the `develop` branch** - _not_ `main`. All PRs should be made against `develop`.
3. Create a new feature branch **off of the `experimental` or `develop` branches** - _not_ `main`. All PRs should be made against either `experimental` or `develop`.
4. Add appropiate documentation, tests, etc, if necessary.
5. Ensure you have your code formatters properly configured (both Prettier and Rustfmt).
6. Once you've completed your changes, create your PR!
7. Follow the PR naming format to help ensure the changelog generator properly picks up your additions:
7. Follow the PR naming format outlined at [gitmoji.dev](https://gitmoji.dev/specification), used for more uniform generation of release notes

> :information_source: Honestly, don't stress about this part right now. I don't even have a changelog generator!! This kind of structure will only matter once releases are more regular and providing changelogs are more important. For now, just make sure your PR name and body is descriptive and concise :heart:
```
<type>: <description>
```

Where `type` is one of the following:

- `feat`: A new feature
- `fix`: A bug fix
- `docs`: Documentation only changes
- `refactor`: A code change that neither fixes a bug nor adds a feature
- `perf`: A code change that improves performance
- `test`: Adding missing tests or correcting existing tests
- `ci`: Changes to our CI configuration files and scripts
- `chore`: Other changes that don't modify `src` or `test` files, such as updating `package.json` or `README.md`
- `revert`: Reverts a previous commit
- `WIP`: Work in progress

The `description` should contain a _succinct_ description of the change:

- use the imperative, present tense: "change" not "changed" nor "changes"
- don't capitalize the first letter
- no dot (.) at the end

Examples:

```
feat: add support for Reading Lists
fix: remove broken link
docs: update CONTRIBUTING.md to include PR naming format
```
> :information_source: Don't stress too much about this part. Just make sure your PR name and body is descriptive and concise, :heart:
8. Stick around and make sure your PR passes all checks and gets merged!

## Issues

I use GitHub issues to track bugs, feature requests, and other tasks. No rigid structure is enforced, but please try and follow these guidelines:
I use GitHub issues to track bugs, feature requests, and other tasks. No rigid structure is enforced, but please try to fill out the templates fully as best you can. Generally, it is useful to include the following information:

- Docker tag (or commit hash displayed in settings)
- Log output (server logs, browser console, etc)
- Access method (browser on host machine, mobile on network, etc)
- Network logs (network tab) and details (reverse proxy, VPN, etc)

If you're not sure if an issue is relevant or appropriate, e.g. if you have more of a question to ask, feel free to pop in the [Discord](https://discord.gg/63Ybb7J3as) and ask!

- Please try and be as descriptive as possible when opening an issue.
- There are a few templates available to help guide you, but if you're not sure which one to use just use the "Blank Issue" template.
- If you're opening an issue to request a feature, please try and explain why you think it would be a good addition to the project. If applicable, include example use cases.
- If you're opening an issue to report a bug, please try and include a minimal reproduction of the bug (video, code, logs, etc).
- If you're not sure if an issue is relevant or appropriate, e.g. if you have more of a question to ask, feel free to pop in the [Discord](https://discord.gg/63Ybb7J3as) and ask!
- **Please don't ghost an issue you've been assigned** - if you're no longer interested in working on it, that is totally okay! Just leave a comment on the issue so that I know you're no longer interested and I can reassign it to someone else. I will never be offended if you no longer want to work on an issue - I'm just trying to make sure that nobody's time and effort is wasted.
## Pull Requests

## A note on merging
> :information_source: There are two development branches: `experimental` and `develop`. These correspond to the `experimental` and `nightly` tags on Docker Hub, respectively. In general, `experimental` is for large or breaking changes, while `develop` is for smaller, more incremental changes.
PRs will be merged once the following criteria are met:

- All CI checks pass
- At least one _maintainer_ has reviewed your PR

All PRs to `develop` will be squashed. All PRs to `main` will be merge commits. This is to ensure that the commit history is clean and easy to follow, and to ensure that the changelog generator works properly.
All PRs to `experimental` will be squashed. All PRs to `develop` from `experimental` and to `main` will be merge commits. This is to ensure that the commit history is clean and easy to follow, and to ensure that the changelog generator works properly.

Thanks for considering contributing to Stump! :heart:
8 changes: 4 additions & 4 deletions README.md
Expand Up @@ -129,14 +129,14 @@ And that's it!

#### Where to start?

If you aren't sure where to start, I recommend taking a look at [open issues](https://github.com/stumpapp/stump/issues). You can also check out the [milestones](https://github.com/stumpapp/stump/milestones) page for a more curated list of issues that need to be addressed.
If you aren't sure where to start, I recommend taking a look at [open issues](https://github.com/stumpapp/stump/issues). You can also check out the [current project board](https://github.com/orgs/stumpapp/projects/4) to see what's actively being worked on or planned.

In general, the following areas are good places to start:

- Translation, so Stump is accessible to as many people as possible
- [Crowdin](https://crowdin.com/project/stump) is being used for translations
- [Crowdin](https://crowdin.com/project/stump) is used for translations
- Writing comprehensive tests
- Designing UI elements/sections or improving the existing UI/UX
- Designing and/improving UI/UX
- Docker build optimizations, caching, etc
- CI pipelines, automated releases and release notes, etc
- And lots more!
Expand All @@ -155,7 +155,7 @@ Stump has a monorepo structure managed by [yarn workspaces](https://yarnpkg.com/
Stand-alone applications that can be run independently, at `/apps` in the root of the project:

- `desktop`: A React + Tauri desktop application
- `mobile`: A React Native application ([#125](https://github.com/stumpapp/stump/issues/125))
- `expo`: A React Native application ([#125](https://github.com/stumpapp/stump/issues/125))
- `server`: An [Axum](https://github.com/tokio-rs/axum) HTTP server
- `web`: A React application, the primary UI for both the built-in web app the server serves and the desktop app

Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/http_server.rs
Expand Up @@ -64,7 +64,7 @@ pub async fn run_http_server(config: StumpConfig) -> ServerResult<()> {
let addr = SocketAddr::from(([0, 0, 0, 0], config.port));
tracing::info!("⚡️ Stump HTTP server starting on http://{}", addr);

// TODO: might need to refactor to use https://docs.rs/async-shutdown/latest/async_shutdown/
// TODO: Refactor to use https://docs.rs/async-shutdown/latest/async_shutdown/
let cleanup = || async move {
println!("Initializing graceful shutdown...");

Expand Down
18 changes: 12 additions & 6 deletions apps/server/src/routers/api/v1/user.rs
Expand Up @@ -637,12 +637,14 @@ async fn get_navigation_arrangement(

let user_preferences = db
.user_preferences()
.find_unique(user_preferences::user_id::equals(user.id.clone()))
.find_first(vec![user_preferences::user::is(vec![user::id::equals(
user.id.clone(),
)])])
.exec()
.await?
.ok_or(APIError::NotFound(format!(
"User preferences with id {} not found",
user.id
"User preferences for {} not found",
user.username
)))?;
let user_preferences = UserPreferences::from(user_preferences);

Expand Down Expand Up @@ -672,12 +674,16 @@ async fn update_navigation_arrangement(

let user_preferences = db
.user_preferences()
.find_unique(user_preferences::user_id::equals(user.id.clone()))
// TODO: Really old accounts potentially have users with preferences missing a `user_id`
// assignment. This should be more properly fixed in the future, e.g. by a migration.
.find_first(vec![user_preferences::user::is(vec![user::id::equals(
user.id.clone(),
)])])
.exec()
.await?
.ok_or(APIError::NotFound(format!(
"User preferences with id {} not found",
user.id
"User preferences for {} not found",
user.username
)))?;
let user_preferences = UserPreferences::from(user_preferences);

Expand Down
78 changes: 55 additions & 23 deletions core/src/config/stump_config.rs
@@ -1,8 +1,14 @@
//! Contains the [StumpConfig] struct and related functions for loading and saving configuration
//! values for a Stump application.
//!
//! Note: [StumpConfig] is constructed _before_ tracing is initializing. This is because the
//! configuration is used to determine the log file path and verbosity level. This means that any
//! logging that occurs during the construction of the [StumpConfig] should be done using the
//! standard `println!` or `eprintln!` macros.
use std::{env, path::PathBuf};

use itertools::Itertools;
use serde::{Deserialize, Serialize};
use tracing::debug;

use crate::error::{CoreError, CoreResult};

Expand Down Expand Up @@ -154,7 +160,7 @@ impl StumpConfig {
let toml_content_str = std::fs::read_to_string(stump_toml)?;
let toml_configs = toml::from_str::<PartialStumpConfig>(&toml_content_str)
.map_err(|e| {
tracing::error!(error = ?e, "Failed to parse Stump.toml");
eprintln!("Failed to parse Stump.toml: {}", e);
CoreError::InitializationError(e.to_string())
})?;

Expand All @@ -171,37 +177,29 @@ impl StumpConfig {
if profile == "release" || profile == "debug" {
env_configs.profile = Some(profile);
} else {
debug!("Invalid PROFILE value: {}", profile);
eprintln!("Invalid PROFILE value: {}", profile);
}
}

if let Ok(port) = env::var(PORT_KEY) {
let port_u16 = port.parse::<u16>().map_err(|e| {
tracing::error!(error = ?e, port, "Failed to parse provided STUMP_PORT");
eprintln!("Failed to parse provided STUMP_PORT: {}", e);
CoreError::InitializationError(e.to_string())
})?;
env_configs.port = Some(port_u16);
}

if let Ok(verbosity) = env::var(VERBOSITY_KEY) {
let verbosity_u64 = verbosity.parse::<u64>().map_err(|e| {
tracing::error!(
error = ?e,
verbosity,
"Failed to parse provided STUMP_VERBOSITY"
);
eprintln!("Failed to parse provided STUMP_VERBOSITY: {}", e);
CoreError::InitializationError(e.to_string())
})?;
env_configs.verbosity = Some(verbosity_u64);
}

if let Ok(pretty_logs) = env::var(PRETTY_LOGS_KEY) {
let pretty_logs_bool = pretty_logs.parse::<bool>().map_err(|e| {
tracing::error!(
error = ?e,
pretty_logs,
"Failed to parse provided STUMP_PRETTY_LOGS"
);
eprintln!("Failed to parse provided STUMP_PRETTY_LOGS: {}", e);
CoreError::InitializationError(e.to_string())
})?;
self.pretty_logs = pretty_logs_bool;
Expand Down Expand Up @@ -249,26 +247,24 @@ impl StumpConfig {
if let Ok(session_ttl) = env::var(SESSION_TTL_KEY) {
match session_ttl.parse() {
Ok(val) => env_configs.session_ttl = Some(val),
Err(e) => tracing::error!(?e, "Failed to parse provided SESSION_TTL"),
Err(e) => eprintln!("Failed to parse provided SESSION_TTL: {}", e),
}
}

if let Ok(session_expiry_interval) = env::var(SESSION_EXPIRY_INTERVAL_KEY) {
match session_expiry_interval.parse() {
Ok(val) => env_configs.expired_session_cleanup_interval = Some(val),
Err(e) => tracing::error!(
?e,
"Failed to parse provided SESSION_EXPIRY_CLEANUP_INTERVAL"
Err(e) => eprintln!(
"Failed to parse provided SESSION_EXPIRY_CLEANUP_INTERVAL: {}",
e
),
}
}

if let Ok(scanner_chunk_size) = env::var(SCANNER_CHUNK_SIZE_KEY) {
match scanner_chunk_size.parse() {
Ok(val) => self.scanner_chunk_size = val,
Err(e) => {
tracing::error!(?e, "Failed to parse provided SCANNER_CHUNK_SIZE")
},
Err(e) => eprintln!("Failed to parse provided SCANNER_CHUNK_SIZE: {}", e),
}
}

Expand Down Expand Up @@ -327,7 +323,7 @@ impl StumpConfig {
std::fs::write(
stump_toml.as_path(),
toml::to_string(&self).map_err(|e| {
tracing::error!(error = ?e, "Failed to serialize StumpConfig to toml");
eprintln!("Failed to serialize StumpConfig to toml: {}", e);
CoreError::InitializationError(e.to_string())
})?,
)?;
Expand Down Expand Up @@ -441,7 +437,7 @@ impl PartialStumpConfig {
if profile == "release" || profile == "debug" {
config.profile = profile;
} else {
debug!("Invalid PROFILE value: {}", profile);
eprintln!("Invalid PROFILE value: {}", profile);
}
}

Expand Down Expand Up @@ -672,6 +668,42 @@ mod tests {
.expect("Failed to delete temporary directory");
}

#[test]
fn test_simulate_first_boot() {
env::set_var(PORT_KEY, "1337");
env::set_var(VERBOSITY_KEY, "2");
env::set_var(DISABLE_SWAGGER_KEY, "true");
env::set_var(HASH_COST_KEY, "1");

let tempdir = tempfile::tempdir().expect("Failed to create temporary directory");
// Now we can create a StumpConfig rooted at the temporary directory
let config_dir = tempdir.path().to_string_lossy().to_string();
let generated = StumpConfig::new(config_dir.clone())
.with_config_file()
.expect("Failed to generate StumpConfig from Stump.toml")
.with_environment()
.expect("Failed to generate StumpConfig from environment");

let expected = StumpConfig {
profile: "debug".to_string(),
port: 1337,
verbosity: 2,
pretty_logs: true,
db_path: None,
client_dir: "./dist".to_string(),
config_dir,
allowed_origins: vec![],
pdfium_path: None,
disable_swagger: true,
password_hash_cost: 1,
session_ttl: DEFAULT_SESSION_TTL,
expired_session_cleanup_interval: DEFAULT_SESSION_EXPIRY_CLEANUP_INTERVAL,
scanner_chunk_size: DEFAULT_SCANNER_CHUNK_SIZE,
};

assert_eq!(generated, expected);
}

fn get_mock_config_file() -> String {
let mock_config_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("integration-tests/data/mock-stump.toml");
Expand Down
3 changes: 2 additions & 1 deletion core/src/db/entity/user/permissions.rs
Expand Up @@ -19,6 +19,7 @@ impl From<prisma::age_restriction::Data> for AgeRestriction {
}
}

// TODO: consider separating some of the `manage` permissions into more granular permissions
// TODO: consider adding self:update permission, useful for child accounts
/// Permissions that can be granted to a user. Some permissions are implied by others,
/// and will be automatically granted if the "parent" permission is granted.
Expand Down Expand Up @@ -58,7 +59,6 @@ pub enum UserPermission {
/// Grant access to delete the library (manage library)
#[serde(rename = "library:delete")]
DeleteLibrary,
// TODO: ReadUsers, CreateUsers, ManageUsers
/// Grant access to read users.
///
/// Note that this is explicitly for querying users via user-specific endpoints.
Expand Down Expand Up @@ -143,6 +143,7 @@ impl ToString for UserPermission {
}
}

// TODO: refactor to remove panic :grimace:
impl From<&str> for UserPermission {
fn from(s: &str) -> UserPermission {
match s {
Expand Down
21 changes: 14 additions & 7 deletions docs/pages/guides/access-control/age-restrictions.mdx
Expand Up @@ -3,9 +3,8 @@ import { Callout } from 'nextra-theme-docs'
# Age Restrictions

<Callout emoji="🚧">
This functionality is experimental. Please ensure you properly test any configured age
restrictions to ensure they work as expected with your library **before** you give the restricted
user access to their account.
Please ensure you properly test any configured age restrictions to ensure they work as expected
with your library **before** you give the restricted user access to their account.
</Callout>

Age restrictions are set on a per-user basis, and are used to determine whether or not a user can access a book. For more information on users and user management, see the dedicated [users](/guides/access-control/users) page.
Expand Down Expand Up @@ -40,6 +39,13 @@ The age restriction is located directly within the metadata for a book itself, o

This means that **without metadata, Stump cannot determine whether or not a book is age-allowed**. There are fallback options described below.

#### EPUB

Stump will attempt to parse one of the following from an EPUB file's metadata:

- `typicalAgeRange`: Generally in the format of `[number]-[number]`
- `contentRating`: Subject to the publisher, but generally similar to that you'd find on a movie or TV show

### How does Stump determine whether or not a book is age-allowed?

If a book or a book's series has an age restriction set, Stump will use that age restriction to determine whether or not a user can access it. The comparison done internally is `less than or equal to X number`, meaning that if a user has an age restriction set to `13` and a book is rated to `17`, the user will not be able to access the book. If a user has an age restriction set to `17` and a book is rated to `13`, the user will be able to access the book. In other words, **the age restriction set on the user must be greater than or equal to the age restriction number set on the book or series in order to have access**.
Expand All @@ -53,7 +59,8 @@ Stump doesn't currently support editing metadata directly, but it is planned for

### Other considerations

- Stump doesn't currently support dynamic thumbnails for libraries containing age-restricted books or series. This means that if it happens to be the case that the first book in the first series of a library is age-restricted, the thumbnail for the library **will still be displayed** so long as a user has access to the library. This is planned to be fixed in the future by one of two ways:
1. Settings will be made available to override the thumbnail for a library or series
2. Server owners will be able to associate libraries and series with tags and then set restrictions on a user that would prevent access to specific tags. See the [Tag-based restrictions](#tag-based-restrictions) section for more information.
3. Server owners can exclude users from seeing certain libraries entirely
#### Library thumbnails

If you generated a library thumbnail for a library which coincidentally contains an age-restricted book that is ordered first, the thumbnail will still be displayed so long as a user has access to the library.

You can get around this by setting a the thumbnail to source from a different book, or uploading a custom thumbnail.

0 comments on commit 4beb7b3

Please sign in to comment.