Skip to content

Commit

Permalink
Add delete button to "video details" (#1172)
Browse files Browse the repository at this point in the history
This adds a "delete" button to the "Video details" page which will
remove that event in Tobira and send a `delete` request to Opencast.
While that operation is pending and when it fails on Opencast side, the
event will still be visible on the "My Videos" page marked respectively,
but only there and only as long as it is still present in Opencast.
Tobira's sync code will pick up if it gets eventually deleted, and then
the "pending/ deletion failed" event will be removed completely.

See commits for more technical details.
Should be fine to review commit by commit.

Closes #806
  • Loading branch information
LukasKalbertodt committed Jun 20, 2024
2 parents 1d71b78 + 859f1a2 commit 17dbd08
Show file tree
Hide file tree
Showing 25 changed files with 460 additions and 125 deletions.
2 changes: 2 additions & 0 deletions backend/src/api/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{
config::Config,
db::Transaction,
search,
sync::OcClient,
prelude::*,
};

Expand All @@ -17,6 +18,7 @@ pub(crate) struct Context {
pub(crate) config: Arc<Config>,
pub(crate) jwt: Arc<JwtContext>,
pub(crate) search: Arc<search::Client>,
pub(crate) oc_client: Arc<OcClient>,
}

impl juniper::Context for Context {}
Expand Down
10 changes: 10 additions & 0 deletions backend/src/api/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ pub(crate) enum ApiErrorKind {

/// Some server error out of control of the API user.
InternalServerError,

/// Communication error with Opencast.
OpencastUnavailable,
}

impl ApiErrorKind {
Expand All @@ -39,6 +42,7 @@ impl ApiErrorKind {
Self::InvalidInput => "INVALID_INPUT",
Self::NotAuthorized => "NOT_AUTHORIZED",
Self::InternalServerError => "INTERNAL_SERVER_ERROR",
Self::OpencastUnavailable => "OPENCAST_UNAVAILABLE",
}
}

Expand All @@ -47,6 +51,7 @@ impl ApiErrorKind {
Self::InvalidInput => "Invalid input",
Self::NotAuthorized => "Not authorized",
Self::InternalServerError => "Internal server error",
Self::OpencastUnavailable => "Opencast unavailable",
}
}
}
Expand Down Expand Up @@ -130,9 +135,14 @@ macro_rules! not_authorized {
($($t:tt)+) => { $crate::api::err::api_err!(NotAuthorized, $($t)*) };
}

macro_rules! opencast_unavailable {
($($t:tt)+) => { $crate::api::err::api_err!(OpencastUnavailable, $($t)*) };
}

pub(crate) use api_err;
pub(crate) use invalid_input;
pub(crate) use not_authorized;
pub(crate) use opencast_unavailable;


// ===== Helper macro to inspect DbError ==================================================
Expand Down
64 changes: 61 additions & 3 deletions backend/src/api/model/event.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt;

use chrono::{DateTime, Utc};
use hyper::StatusCode;
use postgres_types::ToSql;
use serde::{Serialize, Deserialize};
use tokio_postgres::Row;
Expand Down Expand Up @@ -39,6 +40,7 @@ pub(crate) struct AuthorizedEvent {
write_roles: Vec<String>,

synced_data: Option<SyncedEventData>,
tobira_deletion_timestamp: Option<DateTime<Utc>>,
}

#[derive(Debug)]
Expand All @@ -64,6 +66,7 @@ impl_from_db!(
created, updated, start_time, end_time,
tracks, captions, segments,
read_roles, write_roles,
tobira_deletion_timestamp,
},
},
|row| {
Expand All @@ -79,6 +82,7 @@ impl_from_db!(
metadata: row.metadata(),
read_roles: row.read_roles::<Vec<String>>(),
write_roles: row.write_roles::<Vec<String>>(),
tobira_deletion_timestamp: row.tobira_deletion_timestamp(),
synced_data: match row.state::<EventState>() {
EventState::Ready => Some(SyncedEventData {
updated: row.updated(),
Expand Down Expand Up @@ -205,6 +209,10 @@ impl AuthorizedEvent {
context.auth.overlaps_roles(&self.write_roles)
}

fn tobira_deletion_timestamp(&self) -> &Option<DateTime<Utc>> {
&self.tobira_deletion_timestamp
}

async fn series(&self, context: &Context) -> ApiResult<Option<Series>> {
if let Some(series) = self.series {
Ok(Series::load_by_key(series, context).await?)
Expand Down Expand Up @@ -319,6 +327,50 @@ impl AuthorizedEvent {
.pipe(Ok)
}

pub(crate) async fn delete(id: Id, context: &Context) -> ApiResult<RemovedEvent> {
let event = Self::load_by_id(id, context)
.await?
.ok_or_else(|| err::invalid_input!(
key = "event.delete.not-found",
"event not found",
))?
.into_result()?;

if !context.auth.overlaps_roles(&event.write_roles) {
return Err(err::not_authorized!(
key = "event.delete.not-allowed",
"you are not allowed to delete this event",
));
}

let response = context
.oc_client
.delete_event(&event.opencast_id)
.await
.map_err(|e| {
error!("Failed to send delete request: {}", e);
err::opencast_unavailable!("Failed to communicate with Opencast")
})?;

if response.status() == StatusCode::ACCEPTED {
// 202: The retraction of publications has started.
info!(event_id = %id, "Requested deletion of event");
context.db.execute("\
update all_events \
set tobira_deletion_timestamp = current_timestamp \
where id = $1 \
", &[&event.key]).await?;
Ok(RemovedEvent { id })
} else {
warn!(
event_id = %id,
"Failed to delete event, OC returned status: {}",
response.status()
);
Err(err::opencast_unavailable!("Opencast API error: {}", response.status()))
}
}

pub(crate) async fn load_writable_for_user(
context: &Context,
order: EventSortOrder,
Expand Down Expand Up @@ -406,13 +458,13 @@ impl AuthorizedEvent {
select {event_cols}, \
row_number() over(order by ({sort_col}, id) {sort_order}) as row_num, \
count(*) over() as total_count \
from events \
from all_events as events \
{acl_filter} \
order by ({sort_col}, id) {sort_order} \
) as tmp \
{filter} \
limit {limit}",
event_cols = Self::select().with_omitted_table_prefix("events"),
event_cols = Self::select(),
sort_col = order.column.to_sql(),
sort_order = sql_sort_order.to_sql(),
limit = limit,
Expand Down Expand Up @@ -448,7 +500,7 @@ impl AuthorizedEvent {
let total_count = match total_count {
Some(c) => c,
None => {
let query = format!("select count(*) from events {}", acl_filter);
let query = format!("select count(*) from all_events {}", acl_filter);
context.db
.query_one(&query, &[&context.auth.roles_vec()])
.await?
Expand Down Expand Up @@ -669,3 +721,9 @@ pub(crate) struct EventPageInfo {
/// The index of the last returned event.
pub(crate) end_index: Option<i32>,
}

#[derive(juniper::GraphQLObject)]
#[graphql(Context = Context)]
pub(crate) struct RemovedEvent {
id: Id,
}
4 changes: 3 additions & 1 deletion backend/src/api/model/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ impl User {
}

/// Returns all events that somehow "belong" to the user, i.e. that appear
/// on the "my videos" page.
/// on the "my videos" page. This also returns events that have been marked
/// as deleted (meaning their deletion in Opencast has been requested but they
/// are not yet removed from Tobira's database).
///
/// Exactly one of `first` and `last` must be set!
#[graphql(arguments(order(default = Default::default())))]
Expand Down
18 changes: 17 additions & 1 deletion backend/src/api/mutation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use juniper::graphql_object;

use crate::auth::AuthContext;
use crate::{
api::model::event::RemovedEvent,
auth::AuthContext,
};
use super::{
Context,
err::{ApiResult, invalid_input, not_authorized},
Expand Down Expand Up @@ -33,6 +36,7 @@ use super::{
VideoListOrder,
VideoListLayout,
},
event::AuthorizedEvent,
},
};

Expand All @@ -52,6 +56,18 @@ impl Mutation {
Realm::create_user_realm(context).await
}

/// Deletes the given event. Meaning: a deletion request is sent to Opencast, the event
/// is marked as "deletion pending" in Tobira, and fully removed once Opencast
/// finished deleting the event.
///
/// Returns the deletion timestamp in case of success and errors otherwise.
/// Note that "success" in this case only means the request was successfully sent
/// and accepted, not that the deletion itself succeeded, which is instead checked
/// in subsequent harvesting results.
async fn delete_video(id: Id, context: &Context) -> ApiResult<RemovedEvent> {
AuthorizedEvent::delete(id, context).await
}

/// Sets the order of all children of a specific realm.
///
/// `childIndices` must contain at least one element, i.e. do not call this
Expand Down
1 change: 1 addition & 0 deletions backend/src/db/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,4 +366,5 @@ static MIGRATIONS: Lazy<BTreeMap<u64, Migration>> = include_migrations![
31: "series-metadata",
32: "custom-actions",
33: "event-slide-text-and-segments",
34: "event-view-and-deletion-timestamp",
];
17 changes: 17 additions & 0 deletions backend/src/db/migrations/34-event-view-and-deletion-timestamp.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- This adds a 'tobira_deletion_timestamp' column to events to mark videos
-- that have been deleted but are either waiting for sync or still present
-- in Opencast due to a failed deletion on that end. It can be used to
-- detect these failed deletions by comparing it to the current time.

-- Furthermore, the 'events' table is renamed to 'all_events', and a new view
-- called 'events' is created to show all non-deleted records from 'all_events'.
-- This view practically replaces the former 'events' table and removes the
-- need to adjust all queries to check it an event has been deleted.

alter table events
add column tobira_deletion_timestamp timestamp with time zone;

alter table events rename to all_events;

create view events as
select * from all_events where tobira_deletion_timestamp is null;
5 changes: 4 additions & 1 deletion backend/src/http/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ fn frontend_config(config: &Config) -> serde_json::Value {
"small": config.theme.logo.small.as_ref().map(logo_obj),
"largeDark": config.theme.logo.large_dark.as_ref().map(logo_obj),
"smallDark": config.theme.logo.small_dark.as_ref().map(logo_obj),
}
},
"sync": {
"pollPeriod": config.sync.poll_period.as_secs_f64(),
},
})
}
1 change: 1 addition & 0 deletions backend/src/http/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ async fn handle_api(req: Request<Incoming>, ctx: &Context) -> Result<Response, R
config: ctx.config.clone(),
jwt: ctx.jwt.clone(),
search: ctx.search.clone(),
oc_client: ctx.oc_client.clone(),
});
let gql_result = juniper::execute(
&gql_request.query,
Expand Down
5 changes: 4 additions & 1 deletion backend/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::{
metrics,
prelude::*,
search,
sync::OcClient,
util::{self, ByteBody, HttpsClient, UdxHttpClient},
};
use self::{
Expand Down Expand Up @@ -78,6 +79,7 @@ pub(crate) struct Context {
pub(crate) auth_caches: auth::Caches,
pub(crate) http_client: HttpsClient<ByteBody>,
pub(crate) uds_http_client: UdxHttpClient<ByteBody>,
pub(crate) oc_client: Arc<OcClient>,
}


Expand All @@ -96,12 +98,13 @@ pub(crate) async fn serve(
db_pool: db,
assets,
jwt: Arc::new(JwtContext::new(&config.auth.jwt)?),
config: Arc::new(config),
search: Arc::new(search),
metrics: Arc::new(metrics::Metrics::new()),
auth_caches: auth::Caches::new(),
http_client: util::http_client().context("failed to create HTTP client")?,
uds_http_client: UdxHttpClient::unix(),
oc_client: Arc::new(OcClient::new(&config).context("Failed to create Opencast client")?),
config: Arc::new(config),
});

let ctx_clone = ctx.clone();
Expand Down
10 changes: 10 additions & 0 deletions backend/src/sync/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,16 @@ impl OcClient {
self.http_client.request(req).await.map_err(Into::into)
}

pub async fn delete_event(&self, oc_id: &String) -> Result<Response<Incoming>> {
let pq = format!("/api/events/{}", oc_id);
let req = self.req_builder(&pq)
.method(http::Method::DELETE)
.body(RequestBody::empty())
.expect("failed to build request");

self.http_client.request(req).await.map_err(Into::into)
}

fn build_req(&self, path_and_query: &str) -> (Uri, Request<RequestBody>) {
let req = self.req_builder(path_and_query)
.body(RequestBody::empty())
Expand Down
4 changes: 2 additions & 2 deletions backend/src/sync/harvest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ async fn store_in_db(
let segments = segments.into_iter().map(Into::into).collect::<Vec<EventSegment>>();

// We upsert the event data.
upsert(db, "events", "opencast_id", &[
upsert(db, "all_events", "opencast_id", &[
("opencast_id", &opencast_id),
("state", &EventState::Ready),
("series", &series_id),
Expand Down Expand Up @@ -219,7 +219,7 @@ async fn store_in_db(

HarvestItem::EventDeleted { id: opencast_id, .. } => {
let rows_affected = db
.execute("delete from events where opencast_id = $1", &[&opencast_id])
.execute("delete from all_events where opencast_id = $1", &[&opencast_id])
.await?;
check_affected_rows_removed(rows_affected, "event", &opencast_id);
removed_events += 1;
Expand Down
2 changes: 1 addition & 1 deletion backend/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub(crate) struct SyncConfig {
/// The duration to wait after a "no new data" reply from Opencast. Only
/// relevant in `--daemon` mode.
#[config(default = "30s", deserialize_with = crate::config::deserialize_duration)]
poll_period: Duration,
pub(crate) poll_period: Duration,
}


Expand Down
5 changes: 5 additions & 0 deletions frontend/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Config = {
plyr: PlyrConfig;
upload: UploadConfig;
paellaPluginConfig: object;
sync: SyncConfig;
};

type FooterLink = "about" | "graphiql" | {
Expand Down Expand Up @@ -96,6 +97,10 @@ type UploadConfig = {
requireSeries: boolean;
};

type SyncConfig = {
pollPeriod: number;
};

type MetadataLabel = "builtin:license" | "builtin:source" | TranslatedString;

export type TranslatedString = { en: string } & Record<"de", string | undefined>;
Expand Down
Loading

0 comments on commit 17dbd08

Please sign in to comment.