Generalize form structure for permission requests #315

GhiaraD · 2021-10-09T22:21:27Z

Bring the permission request form to the same structure as the class feedback form.
Additional changes:

fix bug where the cancel button for rewriting permission requests doesn't work;
fixed 'Report sent' strings in ro for app feedback ;
users can no longer access the page for requesting permissions if they already have permissions.

page reloads on button press

change how data is extracted from firebase add revert button functionality

* reduce logic at ui level * add 'accepted' and 'processedBy' fields for requests * refactor and format some code for more clarity

…into admin_pannel

* change date place to bottom left corner * change 'deny' bottom colors * add test for admin page * reload only one card instead of all

# Conflicts: # pubspec.yaml

* sort request descending by date * update pubspec.yaml

* update admin panel integration test * resolve warnings

* delete unnecessary mocks * fix Admin Page test

…as well.

* add test for Admin Panel button * fix typos * reduce duplicated code

…o admin_pannel

# Conflicts: # CONTRIBUTING.md # firestore.indexes.json # firestore.rules

…_forms # Conflicts: # firestore.rules # lib/generated/l10n.dart # lib/main.dart # lib/pages/settings/model/request.dart # lib/pages/settings/service/admin_provider.dart # lib/pages/settings/service/request_provider.dart # lib/pages/settings/view/request_card.dart # pubspec.yaml # test/integration_test.dart # test/settings_test.dart

# Conflicts: # lib/main.dart # lib/pages/settings/view/request_card.dart # test/settings_test.dart

Fix test errors

acs-upb-mobile-bot · 2021-10-09T22:23:49Z

	1 Error
🚫	The pubspec.yaml file was updated, but not the version.

	1 Warning
⚠️	Big PR

For more info on versioning, check out our PR guidelines.

Generated by 🚫 Danger

…ions Fix app feedback strings

Fix response not appearing on cards Format code

IoanaAlexandru

This is a good start, but we can make some improvements.

IoanaAlexandru · 2021-10-16T10:38:37Z

firestore.rules

@@ -126,7 +126,7 @@ service cloud.firestore {
        match /forms/{form=**} {
            allow read: if true;
            allow create: if isAuthenticated();
-            allow update: if isAdmin();


Could we at least have something like isAdmin() || isOwner()? Rather than let any authenticated user edit any document.

All requests are in the same document so everyone should be able to update, to create requests in the first place. A more thorough check is rather complicated.

IoanaAlexandru · 2021-10-16T10:39:57Z

lib/l10n/intl_ro.arb

@@ -303,8 +303,9 @@
  "messageAlreadySeeingCurrentWeek": "Ești deja la săptămâna curentă!",
  "messageAnotherQuestion": "Ai altă întrebare?",
  "messageTalkToChatbot": "Vorbește cu Polly!",
-  "messageReportSent": "Reposrt sent successfully.",
-  "messageReportNotSent": "Reposrt could not be sent.",
+  "messageReportSent": "Răspuns trimis cu succes.",


Good catch, I actually asked Alex to fix this (#316) so you'll have to resolve some conflicts when you update your branch.

IoanaAlexandru · 2021-10-16T10:42:21Z

lib/pages/class_feedback/model/form_answer.dart

+  final String questionNumber;
+
+  Map<String, dynamic> toData() {
+    return {};


Don't leave this empty; even if we're not using it, it's a good idea to have a stub to fall back to when inheriting (e.g. if we forget to override). It should be a map with the answer & number.

IoanaAlexandru · 2021-10-16T10:48:08Z

lib/pages/class_feedback/service/feedback_provider.dart

@@ -82,14 +78,14 @@ class FeedbackProvider with ChangeNotifier {
    }
  }

-  Future<Map<String, FeedbackQuestion>> fetchQuestions() async {
+  Future<Map<String, FormQuestion>> fetchQuestions(String document) async {


WDYT about making this public, and adding two helper methods like fetchFeedbackQuestions and fetchPermissionRequestQuestions? And similar to other functions that take the document name.

The reason this would be useful is to not require the UI part to know anything about the database structure. We can just have string constants in this file with the names of the documents and use them directly, so if we at some point rename these documents we only need to change them in one place.

IoanaAlexandru · 2021-10-16T10:50:36Z

lib/pages/class_feedback/service/feedback_provider.dart

@@ -239,4 +237,47 @@ class FeedbackProvider with ChangeNotifier {
      return null;
    }
  }
+
+  bool userAlreadyRequestedCache;


Maybe move to the top of the class for consistency with other providers

IoanaAlexandru · 2021-10-16T11:29:08Z

lib/pages/settings/service/admin_provider.dart

-        'done': true,
-        'accepted': accepted
+      await _db.collection('forms').doc('permission_request_answers').update({
+        '$requestId.processedBy': _authProvider.uid,


Didn't know this syntax is a thing, this is pretty cool!

IoanaAlexandru · 2021-10-16T11:34:12Z

lib/pages/settings/view/request_permissions.dart

@@ -45,7 +50,7 @@ class _RequestPermissionsPageState extends State<RequestPermissionsPage> {
            color: Theme.of(context).accentColor,
            width: 130,
            onTap: () async {
-              Navigator.of(context).pop();
+              await _sendRequest();


Nit: This actually doesn't only send the request, but also pops the page. It would be a bit cleaner if you made _sendRequest return a bool (true if success), and then do:

final res = await _sendRequest(); if (!res) { AppToast... } else { AppToast... Navigator.pop... }

IoanaAlexandru · 2021-10-16T11:35:25Z

lib/pages/settings/view/request_permissions.dart

    _fetchUser();
  }

+  Future<Map<String, dynamic>> fetchFeedbackQuestions() async {


You forgot to change the name

IoanaAlexandru · 2021-10-16T11:35:36Z

lib/pages/settings/view/request_permissions.dart

+        .fetchQuestions('permission_request_questions')
+        .then((questions) => setState(() => requestQuestions = questions));
+    for (int i = 0; i <= requestQuestions.length; i++) {
+      answerValues.insert(i, {


I don't understand what this does

This initializes this 'andswerValues' field, which is specific to questions that require a rating. We probably won't use this type of question here though, I will delete this.

IoanaAlexandru · 2021-10-16T11:37:14Z

lib/pages/settings/view/request_permissions.dart

  @override
  Widget build(BuildContext context) {
-    final requestProvider = Provider.of<RequestProvider>(context);
+    final requestProvider = Provider.of<FeedbackProvider>(context);


This is the same code as on the feedback page, no? Why not make a reusable widget like FeedbackPage now that we have common logic?

It isn't really the same from what I can see. So I think it is better to keep them separated.

# Conflicts: # lib/generated/intl/messages_ro.dart # lib/l10n/intl_ro.arb

GhiaraD and others added 30 commits July 16, 2021 00:46

UI for page where admins can see permission requests

78f814c

accept/deny request buttons

4625a79

v1.0

a627688

page reloads on button press

v2.0

5c8f35d

change how data is extracted from firebase add revert button functionality

add unprocessed filter

8ca56b2

add accepted/denied on processed requests

46331f8

order requests by date, change the place date appears on card

94acdb8

resolve PR change requests

90c1f75

* reduce logic at ui level * add 'accepted' and 'processedBy' fields for requests * refactor and format some code for more clarity

Merge branch 'master' of https://github.com/student-hub/acs-upb-mobile …

70047e9

…into admin_pannel

change version and add changelog

c284f41

Change UI for cards

c193fc5

* change date place to bottom left corner * change 'deny' bottom colors * add test for admin page * reload only one card instead of all

Merge branch 'master' into admin_pannel

5490085

# Conflicts: # pubspec.yaml

Sort requests descending by date

39352e0

* sort request descending by date * update pubspec.yaml

Update integration test

b47549a

* update admin panel integration test * resolve warnings

Format code

530784c

Fix integration test

6a52317

* delete unnecessary mocks * fix Admin Page test

Merge branch 'master' into admin_pannel

3ff5644

Merge changes from master

27f73c4

Set up firebase CLI with firestore indexes & rules.

55c8cee

Add firestore deployment step.

3561bb8

Update functions.

98f738a

Set up storage rules.

d539efc

Update documentation.

7b41233

Use "bash" for command blocks instead of "cli".

f8bd6e6

Use bash syntax highlighting for the other command block in the file …

bd861b5

…as well.

Replace "as close to possible" with "as close as possible".

e632f77

Fix change requests

9d04f2a

* add test for Admin Panel button * fix typos * reduce duplicated code

Merge branch 'master' into admin_pannel

fa8b1a3

Merge remote-tracking branch 'origin/IoanaAlexandru/deploy_rules' int…

3e94e53

…o admin_pannel

Update firebase rules and indexes

84e6a85

GhiaraD added 13 commits August 15, 2021 23:18

Update version

b3ceb59

Merge branch 'master' into admin_pannel

5e3d9b3

# Conflicts: # CONTRIBUTING.md # firestore.indexes.json # firestore.rules

Create more generic classes for forms

5fdd658

Make a function to add each response separately

d60fca0

Write permission requests by user in db

61f3ca3

Get questions from firebase

ade164b

Fix verification if user already made a request

e5db163

Make admin panel read requests from the new place in db 1

9c38a8d

Merge branch 'master' into GhiaraD/restructure_forms

9a24923

# Conflicts: # lib/main.dart # lib/pages/settings/view/request_card.dart # test/settings_test.dart

Bring admin panel to the new form structure

1ab80f6

Fix test errors

Fix bug where cancel button of AppDialog does nothing

041919f

Delete extra changelogs

3075f0a

GhiaraD linked an issue Oct 9, 2021 that may be closed by this pull request

Cancel new permission request #275

Open

GhiaraD added 2 commits October 10, 2021 01:38

Don't let users send permission requests if they already have permiss…

3e96dd2

…ions Fix app feedback strings

Fix tests

a9ada27

Fix response not appearing on cards Format code

GhiaraD requested a review from IoanaAlexandru October 10, 2021 14:04

GhiaraD mentioned this pull request Oct 11, 2021

Update Firestore information in CONTRIBUTING.md #trivial #313

Open

IoanaAlexandru requested changes Oct 16, 2021

View reviewed changes

GhiaraD added 5 commits October 20, 2021 00:39

Remove unused code

6d1a031

Merge branch 'master' into GhiaraD/restructure_forms

988b0fe

# Conflicts: # lib/generated/intl/messages_ro.dart # lib/l10n/intl_ro.arb

Changes left from 9 months ago

cdb3837

Merge branch 'master' into GhiaraD/restructure_forms

8ae324f

Fix typo, rewrite list operation

2aa52c0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalize form structure for permission requests #315

Generalize form structure for permission requests #315

GhiaraD commented Oct 9, 2021 •

edited

Loading

acs-upb-mobile-bot commented Oct 9, 2021 •

edited

Loading

IoanaAlexandru left a comment

IoanaAlexandru Oct 16, 2021

GhiaraD Oct 31, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

IoanaAlexandru Oct 16, 2021

GhiaraD Oct 31, 2021

IoanaAlexandru Oct 16, 2021

GhiaraD May 21, 2022

Generalize form structure for permission requests #315

Are you sure you want to change the base?

Generalize form structure for permission requests #315

Conversation

GhiaraD commented Oct 9, 2021 • edited Loading

acs-upb-mobile-bot commented Oct 9, 2021 • edited Loading

IoanaAlexandru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GhiaraD commented Oct 9, 2021 •

edited

Loading

acs-upb-mobile-bot commented Oct 9, 2021 •

edited

Loading