Skip to content

Commit

Permalink
Consent.fromUserId and createdBySiteAdmin cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
isoos committed Jun 14, 2024
1 parent bb8f83a commit 8085117
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 42 deletions.
6 changes: 0 additions & 6 deletions app/lib/account/consent_backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,9 @@ class ConsentBackend {
// Create a new entry.
final consent = Consent.init(
fromAgent: activeAgent.agentId,
fromUserId: activeUser.userId,
email: email,
kind: kind,
args: args,
createdBySiteAdmin: activeAgent is SupportAgent,
);
await _db.commit(inserts: [
consent,
Expand Down Expand Up @@ -328,7 +326,6 @@ class _PackageUploaderAction extends ConsentAction {
@override
Future<void> onAccept(Consent consent) async {
final packageName = consent.args![0];
final createdBySiteAdmin = consent.createdBySiteAdmin ?? false;
final currentUser = await requireAuthenticatedWebUser();
if (currentUser.email?.toLowerCase() != consent.email?.toLowerCase()) {
throw NotAcceptableException(
Expand All @@ -339,7 +336,6 @@ class _PackageUploaderAction extends ConsentAction {
packageName,
currentUser.user,
consentRequestFromAgent: consent.fromAgent!,
consentRequestCreatedBySiteAdmin: createdBySiteAdmin,
);
}

Expand Down Expand Up @@ -405,7 +401,6 @@ class _PublisherContactAction extends ConsentAction {
publisherId,
contactEmail,
consentRequestFromAgent: consent.fromAgent!,
consentRequestCreatedBySiteAdmin: consent.createdBySiteAdmin ?? false,
);
}

Expand Down Expand Up @@ -485,7 +480,6 @@ class _PublisherMemberAction extends ConsentAction {
publisherId,
currentUser.userId,
consentRequestFromAgent: consent.fromAgent!,
consentRequestCreatedBySiteAdmin: consent.createdBySiteAdmin ?? false,
);
}

Expand Down
13 changes: 0 additions & 13 deletions app/lib/account/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,7 @@ class Consent extends db.Model {
@db.StringListProperty()
List<String>? args;

@db.StringProperty()
String? fromUserId;

/// May be an `User.userId` or `support@pub.dev`.
/// TODO: Migrate to use [fromAgent] instead of [fromUserId].
/// As [Consent] expires after a week, we don't need to wait for long
/// to migrate this in multiple releases. Once the current release is
/// stable for more then a week, we are safe to migrate the code to use
/// the new field and remove the old one.
@db.StringProperty()
String? fromAgent;

Expand All @@ -384,18 +376,13 @@ class Consent extends db.Model {
@db.IntProperty()
int notificationCount = 0;

@db.BoolProperty()
bool? createdBySiteAdmin = false;

Consent();

Consent.init({
required this.fromUserId,
required this.fromAgent,
required this.email,
required this.kind,
required this.args,
this.createdBySiteAdmin = false,
Duration timeout = const Duration(days: 7),
}) {
id = Ulid().toString();
Expand Down
4 changes: 1 addition & 3 deletions app/lib/package/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1497,14 +1497,12 @@ class PackageBackend {
String packageName,
User uploader, {
required String consentRequestFromAgent,
required bool consentRequestCreatedBySiteAdmin,
}) async {
await withRetryTransaction(db, (tx) async {
final packageKey = db.emptyKey.append(Package, id: packageName);
final package = (await tx.lookup([packageKey])).first as Package;

if (!consentRequestCreatedBySiteAdmin &&
consentRequestFromAgent != KnownAgents.pubSupport) {
if (consentRequestFromAgent != KnownAgents.pubSupport) {
await _validatePackageUploader(
packageName,
package,
Expand Down
8 changes: 2 additions & 6 deletions app/lib/publisher/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,9 @@ class PublisherBackend {
String publisherId,
String contactEmail, {
required String consentRequestFromAgent,
required bool consentRequestCreatedBySiteAdmin,
}) async {
checkPublisherIdParam(publisherId);
if (!consentRequestCreatedBySiteAdmin &&
consentRequestFromAgent != KnownAgents.pubSupport) {
if (consentRequestFromAgent != KnownAgents.pubSupport) {
await requirePublisherAdmin(publisherId, consentRequestFromAgent);
}
final authenticatedUser = await requireAuthenticatedWebUser();
Expand Down Expand Up @@ -548,11 +546,9 @@ class PublisherBackend {
String publisherId,
String userId, {
required String consentRequestFromAgent,
required bool consentRequestCreatedBySiteAdmin,
}) async {
checkPublisherIdParam(publisherId);
if (!consentRequestCreatedBySiteAdmin &&
consentRequestFromAgent != KnownAgents.pubSupport) {
if (consentRequestFromAgent != KnownAgents.pubSupport) {
await requirePublisherAdmin(publisherId, consentRequestFromAgent);
}
final user = await accountBackend.lookupUserById(userId);
Expand Down
1 change: 0 additions & 1 deletion app/lib/shared/user_merger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ class UserMerger {
await withRetryTransaction(_db, (tx) async {
final consent = await tx.lookupValue<Consent>(m.key);
if (consent.fromAgent == fromUserId) {
consent.fromUserId = toUserId;
consent.fromAgent = toUserId;
tx.insert(consent);
}
Expand Down
1 change: 0 additions & 1 deletion app/test/admin/api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,6 @@ void main() {

final consentRow = await dbService.query<Consent>().run().single;
expect(consentRow.args, ['oxygen']);
expect(consentRow.createdBySiteAdmin, isTrue);

await (await createFakeAuthPubApiClient(email: 'someuser@pub.dev'))
.resolveConsent(
Expand Down
2 changes: 1 addition & 1 deletion app/test/frontend/golden/pkg_activity_log_page.html
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ <h3 class="detail-lead-title">Metadata</h3>
<tbody>
<tr>
<td class="date">
<a class="-x-ago" href="" title="%%published-date%%" aria-label="%%x-ago%%" aria-role="button" role="button" data-timestamp="%%millis%%">%%x-ago%%</a>
<a class="-x-ago" href="" title="%%activity-0-date%%" aria-label="%%x-ago%%" aria-role="button" role="button" data-timestamp="%%millis%%">%%x-ago%%</a>
</td>
<td class="summary">
<div class="markdown-body">
Expand Down
5 changes: 0 additions & 5 deletions app/test/publisher/api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ void main() {
.map((c) => {
'id': c.consentId,
'fromAgent': c.fromAgent,
'fromUserId': c.fromUserId,
'email': c.email,
'kind': c.kind,
'args': c.args,
Expand Down Expand Up @@ -402,7 +401,6 @@ void main() {
);
final consent = Consent.init(
fromAgent: adminUser.userId,
fromUserId: adminUser.userId,
email: 'other@pub.dev',
kind: 'PublisherMember',
args: ['example.com'],
Expand All @@ -419,7 +417,6 @@ void main() {
{
'id': isNotNull,
'fromAgent': adminUser.userId,
'fromUserId': adminUser.userId,
'email': 'other@pub.dev',
'kind': 'PublisherMember',
'args': ['example.com'],
Expand Down Expand Up @@ -449,7 +446,6 @@ void main() {
{
'id': isNotNull,
'fromAgent': user.userId,
'fromUserId': user.userId,
'email': 'newuser@example.com',
'kind': 'PublisherMember',
'args': ['example.com'],
Expand All @@ -469,7 +465,6 @@ void main() {
{
'id': isNotNull,
'fromAgent': user.userId,
'fromUserId': user.userId,
'email': 'user@pub.dev',
'kind': 'PublisherMember',
'args': ['example.com'],
Expand Down
9 changes: 3 additions & 6 deletions app/test/shared/user_merger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,18 @@ void main() {
kind: 'k1',
args: ['1'],
fromAgent: user.userId,
fromUserId: user.userId,
);
final target2 = Consent.init(
email: user.email,
kind: 'k2',
args: ['2'],
fromAgent: admin.userId,
fromUserId: admin.userId,
);
final controlConsent = Consent.init(
email: control.email,
kind: 'k3',
args: ['3'],
fromAgent: control.userId,
fromUserId: control.userId,
);
await dbService.commit(inserts: [target1, target2, controlConsent]);

Expand All @@ -123,9 +120,9 @@ void main() {
final updated2 = list.firstWhere((c) => c.id == target2.id);
final updated3 = list.firstWhere((c) => c.id == controlConsent.id);

expect(updated1.fromUserId, user.userId);
expect(updated2.fromUserId, user.userId);
expect(updated3.fromUserId, control.userId);
expect(updated1.fromAgent, user.userId);
expect(updated2.fromAgent, user.userId);
expect(updated3.fromAgent, control.userId);
});

testWithProfile('publisher membership',
Expand Down

0 comments on commit 8085117

Please sign in to comment.