Skip to content

Commit 9c809d9

Browse files
labkey-jeckelslabkey-jeckels
committed
Improved permission checks and test coverage
1 parent 4ffbcd6 commit 9c809d9

File tree

12 files changed

+314
-89
lines changed

12 files changed

+314
-89
lines changed

‎resources/queries/targetedms/InstrumentBilling.sql‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ SELECT
2828
TIMESTAMPDIFF('SQL_TSI_HOUR', StartTime, EndTime) * ir.Fee + ir.rateType.setupFee AS TotalCost,
2929
iup.PaymentMethod.UWBudgetNumber AS Payment_Method,
3030
iup.PaymentMethod.Name AS Payment_Method_Name,
31-
iup.PercentPayment
31+
iup.PercentPayment,
32+
((TIMESTAMPDIFF('SQL_TSI_HOUR', StartTime, EndTime) * Fee + ir.rateType.setupFee) * PercentPayment / 100) AS AmountBilled
3233

3334
FROM targetedms.InstrumentSchedule i
34-
LEFT OUTER JOIN targetedms.InstrumentRate ir ON i.Instrument = ir.Instrument
35-
LEFT OUTER JOIN targetedms.InstrumentUsagePayment iup ON i.Id = iup.InstrumentScheduleId
35+
INNER JOIN targetedms.InstrumentRate ir ON i.Instrument = ir.Instrument
36+
INNER JOIN targetedms.InstrumentUsagePayment iup ON i.Id = iup.InstrumentScheduleId

‎resources/queries/targetedms/InstrumentBillingByMonth.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ SELECT
3131
Payment_Method,
3232
Payment_Method_Name,
3333
PercentPayment,
34-
((HoursInRange * Fee + Setup_Cost) * PercentPayment / 100) AS AmountBilled
34+
-- Only include the setup fee when the beginning of the reservation falls within the report's time window
35+
((HoursInRange * Fee + (CASE WHEN StartDate <= StartBillDate THEN 0 ELSE Setup_Cost END)) * PercentPayment / 100) AS AmountBilled
3536

3637
FROM
3738
(SELECT

‎resources/queries/targetedms/instrumentBilling.query.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22
<metadata>
33
<tables xmlns="http://labkey.org/data/xml">
44
<table tableName="instrumentBilling" tableDbType="TABLE">
5+
<tableTitle>Instrument Billing</tableTitle>
56
<columns>
67
<column columnName="Hours">
78
<formatString>0.0</formatString>
89
</column>
910
<column columnName="TotalCost">
1011
<formatString>$#,##0.00</formatString>
1112
</column>
13+
<column columnName="AmountBilled">
14+
<formatString>$#,##0.00</formatString>
15+
</column>
1216
</columns>
1317
</table>
1418
</tables>

‎resources/queries/targetedms/instrumentBillingByMonth.query.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
<metadata>
33
<tables xmlns="http://labkey.org/data/xml">
44
<table tableName="instrumentBillingByMonth" tableDbType="TABLE">
5+
<tableTitle>Instrument Billing by Month</tableTitle>
56
<columns>
67
<column columnName="HoursInRange">
78
<formatString>0.0</formatString>

‎src/org/labkey/targetedms/TargetedMSSchema.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
import org.labkey.targetedms.query.PeptideTableInfo;
106106
import org.labkey.targetedms.query.PrecursorChromInfoTable;
107107
import org.labkey.targetedms.query.PrecursorTableInfo;
108+
import org.labkey.targetedms.query.ProjectAddUserTrigger;
108109
import org.labkey.targetedms.query.QCAnnotationTable;
109110
import org.labkey.targetedms.query.QCAnnotationTypeTable;
110111
import org.labkey.targetedms.query.QCEnabledMetricsTable;
@@ -1631,7 +1632,7 @@ public DisplayColumn createRenderer(ColumnInfo colInfo)
16311632
TABLE_PROJECT_RESEARCHER.equalsIgnoreCase(name) ||
16321633
TABLE_INSTRUMENT_USAGE_PAYMENT.equalsIgnoreCase(name))
16331634
{
1634-
var result = new OwnProjectSchedulingTable(name, this, cf, TABLE_MS_PROJECT.equalsIgnoreCase(name));
1635+
var result = new OwnProjectSchedulingTable(name, this, cf, !TABLE_MS_PROJECT.equalsIgnoreCase(name));
16351636
if (TABLE_INSTRUMENT_USAGE_PAYMENT.equalsIgnoreCase(name))
16361637
{
16371638
result.addTriggerFactory((c, table, extraContext) -> List.of(new InstrumentUsagePaymentTrigger("InstrumentScheduleId")));
@@ -1654,17 +1655,19 @@ public DisplayColumn createRenderer(ColumnInfo colInfo)
16541655
{
16551656
result.addCondition(projectMemberSql, FieldKey.fromParts("Id"));
16561657
}
1658+
result.addTriggerFactory((c, table, extraContext) -> List.of(new ProjectAddUserTrigger()));
16571659
}
16581660
else if (TABLE_INSTRUMENT_USAGE_PAYMENT.equalsIgnoreCase(name))
16591661
{
16601662
// For non-admins, completely hide payment data for projects they're not associated with
16611663
if (!getContainer().hasPermission(getUser(), AdminPermission.class))
16621664
{
1663-
SQLFragment projectMemberSql = new SQLFragment("EXISTS (SELECT Project FROM ");
1665+
SQLFragment projectMemberSql = new SQLFragment("InstrumentScheduleId IN (SELECT i.Id FROM ");
1666+
projectMemberSql.append(TargetedMSManager.getTableInfoInstrumentSchedule(), "i");
1667+
projectMemberSql.append(" INNER JOIN ");
16641668
projectMemberSql.append(TargetedMSManager.getTableInfoProjectResearcher(), "pr");
1665-
projectMemberSql.append(" WHERE pr.researcher = ? AND pr.project = ");
1669+
projectMemberSql.append(" ON i.Project = pr.Project AND pr.researcher = ?)");
16661670
projectMemberSql.add(getUser().getUserId());
1667-
projectMemberSql.append(ExprColumn.STR_TABLE_ALIAS).append(".Project)");
16681671
result.addCondition(projectMemberSql, FieldKey.fromParts("Project"));
16691672
}
16701673

‎src/org/labkey/targetedms/query/InstrumentScheduleOverlapTrigger.java‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,46 +15,49 @@
1515
import java.util.Map;
1616

1717
/**
18-
* Prevents overlapping instrument schedule entries for the same instrument.
18+
* Prevents overlapping instrument schedule entries for the same instrument and that the instrument is active.
1919
*/
2020
public class InstrumentScheduleOverlapTrigger implements Trigger
2121
{
2222
@Override
23-
public void beforeInsert(TableInfo table, Container c, User user, Map<String, Object> newRow, ValidationException errors, Map<String, Object> extraContext)
23+
public void beforeInsert(TableInfo table, Container c, User user, Map<String, Object> newRow, ValidationException errors, Map<String, Object> extraContext) throws ValidationException
2424
{
25-
validateNoOverlap(newRow, null, errors);
25+
validateNoOverlap(newRow, null);
26+
Number instrument = (Number) newRow.get("Instrument");
27+
SqlSelector selector = new SqlSelector(TargetedMSManager.getSchema(), "SELECT Id FROM " + TargetedMSManager.getTableInfoMSInstrument() + " WHERE Id = ? AND Active = ?", instrument, true);
28+
if (!selector.exists())
29+
{
30+
throw new ValidationException("Instrument does not exist or is not active");
31+
}
2632
}
2733

2834
@Override
29-
public void beforeUpdate(TableInfo table, Container c, User user, @Nullable Map<String, Object> newRow, @Nullable Map<String, Object> oldRow, ValidationException errors, Map<String, Object> extraContext)
35+
public void beforeUpdate(TableInfo table, Container c, User user, @Nullable Map<String, Object> newRow, @Nullable Map<String, Object> oldRow, ValidationException errors, Map<String, Object> extraContext) throws ValidationException
3036
{
3137
// Use the Id from either newRow or oldRow to exclude the current record
3238
Integer id = getId(newRow);
3339
if (id == null)
3440
id = getId(oldRow);
35-
validateNoOverlap(newRow, id, errors);
41+
validateNoOverlap(newRow, id);
3642
}
3743

38-
private void validateNoOverlap(Map<String, Object> row, @Nullable Integer excludeId, ValidationException errors)
44+
private void validateNoOverlap(Map<String, Object> row, @Nullable Integer excludeId) throws ValidationException
3945
{
4046
Number instrument = (Number) row.get("Instrument");
4147
Date start = (Date) row.get("StartTime");
4248
Date end = (Date) row.get("EndTime");
4349

4450
if (instrument == null)
4551
{
46-
errors.addGlobalError("Instrument is required");
47-
return;
52+
throw new ValidationException("Instrument is required");
4853
}
4954
if (start == null || end == null)
5055
{
51-
errors.addGlobalError("StartTime and EndTime are required");
52-
return;
56+
throw new ValidationException("StartTime and EndTime are required");
5357
}
5458
if (!start.before(end))
5559
{
56-
errors.addGlobalError("StartTime must be before EndTime");
57-
return;
60+
throw new ValidationException("StartTime must be before EndTime");
5861
}
5962

6063
// Overlap condition: existing.start < new.end AND existing.end > new.start
@@ -75,7 +78,7 @@ private void validateNoOverlap(Map<String, Object> row, @Nullable Integer exclud
7578
boolean overlapExists = new SqlSelector(TargetedMSManager.getSchema(), sql).exists();
7679
if (overlapExists)
7780
{
78-
errors.addGlobalError("Instrument schedule overlaps with an existing reservation for this instrument");
81+
throw new ValidationException("Instrument schedule overlaps with an existing reservation for this instrument");
7982
}
8083
}
8184

‎src/org/labkey/targetedms/query/InstrumentScheduleTable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class InstrumentScheduleTable extends OwnProjectSchedulingTable
2424
{
2525
public InstrumentScheduleTable(TargetedMSSchema schema, ContainerFilter cf)
2626
{
27-
super(TargetedMSSchema.TABLE_INSTRUMENT_SCHEDULE, schema, cf, false);
27+
super(TargetedMSSchema.TABLE_INSTRUMENT_SCHEDULE, schema, cf, true);
2828
addTriggerFactory((c, table, extraContext) -> List.of(
2929
new InstrumentUsagePaymentTrigger("Id"),
3030
new InstrumentScheduleOverlapTrigger()

‎src/org/labkey/targetedms/query/InstrumentUsagePaymentTrigger.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,26 @@ public void complete(TableInfo table, Container c, User user, TableInfo.TriggerT
8181
{
8282
throw new ApiUsageException("Instrument usage payments do not add up to 100%");
8383
}
84+
85+
for (int scheduleId : _schedulesToCheck)
86+
{
87+
SQLFragment wrongProjectSql = new SQLFragment("SELECT * FROM ");
88+
wrongProjectSql.append(TargetedMSManager.getTableInfoInstrumentUsagePayment(), "iup");
89+
wrongProjectSql.append(" INNER JOIN ");
90+
wrongProjectSql.append(TargetedMSManager.getTableInfoInstrumentSchedule(), "s");
91+
wrongProjectSql.append(" ON iup.InstrumentScheduleId = s.Id\n");
92+
wrongProjectSql.append(" WHERE iup.InstrumentScheduleId = ? AND iup.PaymentMethod NOT IN (SELECT PaymentMethod FROM ");
93+
wrongProjectSql.add(scheduleId);
94+
wrongProjectSql.append(TargetedMSManager.getTableInfoProjectPaymentMethod(), "ppm");
95+
wrongProjectSql.append(" WHERE ppm.Project = s.Project)");
96+
97+
if (new SqlSelector(TargetedMSManager.getSchema(), wrongProjectSql).exists())
98+
{
99+
throw new ApiUsageException("Instrument usage payments are not using a payment method that is configured for the project.");
100+
}
101+
}
102+
103+
84104
}, DbScope.CommitTaskOption.PRECOMMIT);
85105
}
86106
}

‎src/org/labkey/targetedms/query/ProjectAddUserTrigger.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package org.labkey.targetedms.query;
2+
3+
import org.jetbrains.annotations.Nullable;
4+
import org.labkey.api.data.Container;
5+
import org.labkey.api.data.Table;
6+
import org.labkey.api.data.TableInfo;
7+
import org.labkey.api.data.triggers.Trigger;
8+
import org.labkey.api.query.ValidationException;
9+
import org.labkey.api.security.User;
10+
import org.labkey.targetedms.TargetedMSManager;
11+
12+
import java.util.Collection;
13+
import java.util.HashMap;
14+
import java.util.Map;
15+
16+
public class ProjectAddUserTrigger implements Trigger
17+
{
18+
19+
@Override
20+
public void afterInsert(TableInfo table, Container c,
21+
User user, @Nullable Map<String, Object> newRow,
22+
ValidationException errors, Map<String, Object> extraContext, @Nullable Map<String, Object> existingRecord) throws ValidationException
23+
{
24+
// Add the creating user to the membership list
25+
Map<String, Object> membership = new HashMap<>();
26+
membership.put("Project", newRow.get("Id"));
27+
membership.put("Researcher", user.getUserId());
28+
membership.put("Container", c.getId());
29+
30+
Table.insert(user, TargetedMSManager.getTableInfoProjectResearcher(), membership);
31+
}
32+
}

‎src/org/labkey/targetedms/query/SimpleTargetedMSTable.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,17 @@
33
import org.jetbrains.annotations.NotNull;
44
import org.labkey.api.data.ContainerFilter;
55
import org.labkey.api.query.DefaultQueryUpdateService;
6-
import org.labkey.api.query.FilteredTable;
76
import org.labkey.api.query.QueryUpdateService;
7+
import org.labkey.api.query.SimpleUserSchema;
88
import org.labkey.api.security.UserPrincipal;
99
import org.labkey.api.security.permissions.Permission;
1010
import org.labkey.targetedms.TargetedMSSchema;
1111

12-
public class SimpleTargetedMSTable extends FilteredTable<TargetedMSSchema>
12+
public class SimpleTargetedMSTable extends SimpleUserSchema.SimpleTable<TargetedMSSchema>
1313
{
1414
public SimpleTargetedMSTable(String name, TargetedMSSchema schema, ContainerFilter cf)
1515
{
16-
super(TargetedMSSchema.getSchema().getTable(name), schema, cf);
16+
super(schema, TargetedMSSchema.getSchema().getTable(name), cf);
1717
wrapAllColumns(true);
1818
}
1919

0 commit comments

Comments
 (0)