Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account Request Form Upgrade #11878

Closed
fans2619 opened this issue Jun 26, 2022 · 37 comments · Fixed by #13048 or #13051
Closed

Account Request Form Upgrade #11878

fans2619 opened this issue Jun 26, 2022 · 37 comments · Fixed by #13048 or #13051
Assignees
Labels
c.Epic Feature/task that is worth many smaller sub-features/sub-tasks committers only Difficult; better left for committers or more senior developers enhancement New feature or request

Comments

@fans2619
Copy link
Contributor

fans2619 commented Jun 26, 2022

This issue keeps track of PRs relating to the account request form upgrade. More info can be found on the wiki at https://github.com/TEAMMATES/teammates/wiki/Account-Request-Form-Upgrade.

@samuelfangjw samuelfangjw added committers only Difficult; better left for committers or more senior developers c.Epic Feature/task that is worth many smaller sub-features/sub-tasks enhancement New feature or request labels Jun 26, 2022
@fans2619
Copy link
Contributor Author

@damithc Good night! I just want to check a few more about the account request form. When you are adding an instructor currently, I guess you only fill in the three fields for every instructor: name, email, institution. So I wonder how you use other information provided: country, home page URL, comments/queries. Are these three for viewing only and won't be edited or stored currently?

@damithc
Copy link
Contributor

damithc commented Jun 29, 2022

@damithc Good night! I just want to check a few more about the account request form. When you are adding an instructor currently, I guess you only fill in the three fields for every instructor: name, email, institution. So I wonder how you use other information provided: country, home page URL, comments/queries. Are these three for viewing only and won't be edited or stored currently?

@fans2619 I use name | email | institution, country

url and comments/queries are used to decide if the request from a legit instructor i.e., you need to display those two to me.

@fans2619
Copy link
Contributor Author

I use name | email | institution, country

I see. The institution, country seems a little troublesome to deal with. Personally I think you won't need to edit the country field in any way but only the country part in the institution. So I tend to show country again in the pop-up (not editable) and the institution shown can be institution, country (editable). By the way, I just came up with another question: probably we want to keep all the info submitted by a user before it's edited, e.g., original name & edited name.

url and comments/queries are used to decide if the request from a legit instructor i.e., you need to display those two to me.

So url and comments/queries will definitely be shown in a separate pop-up due to space limitation.

@damithc
Copy link
Contributor

damithc commented Jun 29, 2022

I see. The institution, country seems a little troublesome to deal with. Personally I think you won't need to edit the country field in any way but only the country part in the institution. So I tend to show country again in the pop-up (not editable) and the institution shown can be institution, country (editable). By the way, I just came up with another question: probably we want to keep all the info submitted by a user before it's edited, e.g., original name & edited name.

Need both to be editable, if the data is collected as free text. One obvious reason is to fix typos.

Currently, I don't keep the original data (but perhaps the GSheets does that anyway). I'm not sure if it is worth keeping the original data (I can't think of any use).

So url and comments/queries will definitely be shown in a separate pop-up due to space limitation.

That makes my life harder. Figure out a way to show everything without needing further clicks. The current UI can be changed as necessary. It is not very editing-friendly anyway, as you can see below (it's hard to edit when you can see only the first 10 characters of the text):
image

@fans2619
Copy link
Contributor Author

fans2619 commented Jun 29, 2022

Need both to be editable, if the data is collected as free text. One obvious reason is to fix typos.

Okay I think of a way. So I'll provide institution and country separately and a non-editable result showing the combination of the two (institution, country). The result will be updated automatically after you edit either institution or country, and will be used as the final institution.

Currently, I don't keep the original data (but perhaps the GSheets does that anyway). I'm not sure if it is worth keeping the original data (I can't think of any use).

Yup I can't think of any use either but it's just my habit to keep a copy of everything. I don't think it would take too much space in the database, but I'm very fine with not keeping them also, which apparently is easier to implement.

That makes my life harder. Figure out a way to show everything without needing further clicks. The current UI can be changed as necessary. It is not very editing-friendly anyway, as you can see below (it's hard to edit when you can see only the first 10 characters of the text): image

I can show each account request like something below, but this will make the home page very long. So to shorten it, it may be possible to collapse requests, if you are okay with?
image
image

By the way, I just realised something. If we do not use the Google sheet, you probably won't be able to view a past request any more. It doesn't seem to be feasible to fetch and display all the requests every time when the page is loaded. So do you have any way to quickly locate a past request, either through Search or Log, or you seldom search for a past request? Myself is having some trouble with the search function, maybe caused by my setup.

@damithc
Copy link
Contributor

damithc commented Jun 29, 2022

I can show each account request like something below, but this will make the home page very long. So to shorten it, it may be possible to collapse requests, if you are okay with?

Yes, panels are OK. Again, don't make me click on a panel to see the contents (i.e., they should be expanded by default).

By the way, I just realised something. If we do not use the Google sheet, you probably won't be able to view a past request any more. It doesn't seem to be feasible to fetch and display all the requests every time when the page is loaded. So do you have any way to quickly locate a past request, either through Search or Log, or you seldom search for a past request? Myself is having some trouble with the search function, maybe caused by my setup.

Good enough if they are searchable.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

@damithc How does this look like? It's a simple mockup.
image

@damithc
Copy link
Contributor

damithc commented Jul 4, 2022

@damithc How does this look like? It's a simple mockup.
image

Use more realistic data e.g., the actual name/institute is likely a lot longer.

Is this the search results view or account creation page view?

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

This is the search results. Long fields will wrap to the next line.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

This mockup was created manually instead of using html. I think the focus here should be on whether the Status and Options are fine.

@damithc
Copy link
Contributor

damithc commented Jul 4, 2022

This is the search results. Long fields will wrap to the next line.

I see. I assume Approve/Reject buttons are not strictly necessary as those actions are primarily done via the account creation page? If yes, can skip them to keep things simple. But if you do provide them here, the 'Edit' button should be here as well?

What does 'Reset Account Request' mean?

I think the focus here should be on whether the Status and Options are fine.

What does 'Registered' mean? Account created? If so, use 'Created' instead, so as not to confuse it with instructor joining TEAMMATES.

What's the purpose of 'Expand/Collapse All' buttons?

Less important, can be omitted in the first version:

  1. We might need 'Resend' button here, in case the email doesn't go through for some reason.
  2. The Submitted/Rejected/... etc. labels can use different color text (or some other styling) to easily differentiate between them.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

What's the purpose of 'Expand/Collapse All' buttons?

An existing feature. I don't think it's useful though. (By the way, searching for account requests is an existing feature, and I'm just making changes to it.)

What does 'Reset Account Request' mean?

An existing feature again, but let me check it out, because it just came to my mind that instead of providing everything in the search results, probably a "Reset" button is enough to set the status back to "Submitted" so you can process it again on the home page.

What does 'Registered' mean? Account created? If so, use 'Created' instead, so as not to confuse it with instructor joining TEAMMATES.

The term "Registered" is reused from the current (there is a "Registered At" datetime). And "Registered" means an account (not account request) is created. An account is created now when an instructor clicks the "join" link and join the first course. "Created" will be referring to when the account request is created (i.e., when instructor submits the request form). Personally I want to change "Created At" to "Submitted At". However, I think this change can be done last, to reduce the complexity of the data migration. For "Registered", I don't have much opinion on it though. "Joined" may also be a good term to consider.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

  1. We might need 'Resend' button here, in case the email doesn't go through for some reason.

I think having a 'Reset' button will also be able to cover this, right? You can approve it again after reset. By the way, the join link is actually displayed in the panel. It can always be manually copied and sent. (It's called "Account Registration Link" now.)

image

  1. The Submitted/Rejected/... etc. labels can use different color text (or some other styling) to easily differentiate between them.

Sure. That can be done last.

@damithc
Copy link
Contributor

damithc commented Jul 4, 2022

The term "Registered" is reused from the current (there is a "Registered At" datetime). And "Registered" means an account (not account request) is created. An account is created now when an instructor clicks the "join" link and join the first course. "Created" is referring to when the account request is created (i.e., when instructor submits the request form). Personally I want to change "Created At" to "Submitted At". However, I think this change can be done last, to reduce the complexity of the data migration. For "Registered", I don't have much opinion on it though. "Joined" may also be a good term to consider.

Let's keep it as 'Registered'. It keep forgetting that the account is created when the instructor joins, which was not the case for most of the the TEAMMATES history. The flow Submitted -> Approved -> Registered makes sense.

I haven't used the 'Reset Account Request' feature yet. Find out what it does. Can keep if it has a useful purpose.
The expand/collapse all can be kept too. It shows the join-link for each request.

@damithc
Copy link
Contributor

damithc commented Jul 4, 2022

  1. We might need 'Resend' button here, in case the email doesn't go through for some reason.

I think having a 'Reset' button will also be able to cover this, right? You can approve it again after reset. By the way, the join link is actually displayed in the panel. It can always be manually copied and sent. (It's called "Account Registration Link" now.)

Yes, that works.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 4, 2022

Let's keep it as 'Registered'.

Okay!

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 6, 2022

@damithc You may know that there's no country field in the database currently since you always put the country inside institute. As a result, if we want to store institute and country separately, the way of ID generation for an account request needs to be changed. I have actually kind of achieved this separation already, but the solution is very complicated, and it may cause undesired consequence, although it's unlikely if you have been using institute, country all the time so far since we have account request database. The solution is complicated as it requires all the places of identifying an account request to use 3 parameters (email, institute, country) instead of the original 2 parameters (email, institute). This also includes that a new field country needs to be created to the accountrequest core in the Solr search server. So even though this can be achieved, I would still recommend storing institute and country together directly. This means once users submit the form, the two fields will be combined and stored in the database and later on, you will be editing this "final institute" instead of editing both institute and country. For your referece, we store the original institute and country submitted by users as well but there's no need for them to be editable at all. With that said, it's also okay to make them editable, in case they will be used in some way in the future.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 6, 2022

But this can be equivalent to that you edit the institute and country, and we have a combined institute with country stored in the database. I think I'll use this instead. So in the end, the combined still should be stored, in order to make the implementation easier.

@damithc
Copy link
Contributor

damithc commented Jul 6, 2022

@fans2619 I'm fine as long as I can edit both of them after the user has submitted them, when I'm using the admin UI to accept/reject account requests. They don't even have to be presented to me as two separate fields. It is fine if you want to store them a a single field in the database. Does that clear your doubt?

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 6, 2022

Yup thanks prof!

@fans2619
Copy link
Contributor Author

I'm fine as long as I can edit both of them after the user has submitted them, when I'm using the admin UI to accept/reject account requests. They don't even have to be presented to me as two separate fields. It is fine if you want to store them a a single field in the database. Does that clear your doubt?

@damithc I will store them as a single field and throw away the original data submitted. You'll see only one field and edit it directly. This is to save space in the database. However, some cons are that it may make it hard to read if there are "," in the institute and/or country. There's no perfect solution and it seems that saving space and saving cost is more important for us.

@damithc
Copy link
Contributor

damithc commented Jul 11, 2022

@damithc I will store them as a single field and throw away the original data submitted. You'll see only one field and edit it directly. This is to save space in the database. However, some cons are that it may make it hard to read if there are "," in the institute and/or country. There's no perfect solution and it seems that saving space and saving cost is more important for us.

@fans2619 In the case of account requests, the space and cost may not matter much as the number of entities is relatively small. So, you can save as two fields too, if it helps.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 11, 2022

Hmm to be honest, what helps me more is to store as a single field. I just hope that won't make admin's life too difficult. Storing as two fields is the hardest to implement, so it's either 1 (instituteWithCountry only) or 3 (institute, country, instituteWithCountry).

@damithc
Copy link
Contributor

damithc commented Jul 11, 2022

Hmm to be honest, what helps me more is to store as a single field. I just hope that won't make admin's life too difficult. Storing as two fields is the hardest to implement, so it's either 1 (instituteWithCountry only) or 3 (institute, country, instituteWithCountry).

Sure, no objections.

@fans2619
Copy link
Contributor Author

fans2619 commented Jul 13, 2022

@damithc Previously (long time ago) we discussed about what to do when another REJECTED account request exists when a user submits the account request form, and we decided to overwrite that account request. However, thinking about it again last night, it may not be a good enough solution because it can cause admin to process the same request again and again. So what if we just ban all submissions if the email%institute conflicts with an existing request, regardless of the status? And we should tell them a request already exists and contact us if in doubt in this case.

@damithc
Copy link
Contributor

damithc commented Jul 13, 2022

@damithc Previously (long time ago) we discussed about what to do when another REJECTED account request exists when a user submits the account request form, and we decided to overwrite that account request. However, thinking about it again last night, it may not be a good enough solution because it can cause admin to process the same request again and again. So what if we just ban all submissions if the email%institute conflicts with an existing request, regardless of the status? And we should tell them a request already exists and contact us if in doubt in this case.

You mentioned earlier that doing so is a minor data leakage as it allows any third party to find out if a certain other person has requested for a TEAMMATES account.

Would adding another status 'Reapplied' help?

ziqing26 pushed a commit that referenced this issue Apr 15, 2024
* fix tooltip

* re-add removed lines

* update snap

* update snaps

* lint

* suppress warning

* lint
ziqing26 pushed a commit that referenced this issue Apr 15, 2024
…ing email (#13032)

* Add check if admin when creating account request

* Trim down test case
ziqing26 pushed a commit that referenced this issue Apr 16, 2024
* feat: add order-by

* fix: fix failing test case

* fix: add missing created-at field

* fix: test case comment
jayasting98 pushed a commit that referenced this issue Apr 16, 2024
* Add E2E skeleton

* Fix test and lint

* Add verifyEmailSent

* fix fe tests
jayasting98 added a commit that referenced this issue Apr 17, 2024
* Add get typical account request method

* Migrate AccountRequestsLogicTest

* Remove test for get by email address and institute

---------

Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>
ziqing26 pushed a commit that referenced this issue Apr 17, 2024
* add admin e2e tests

* remove exception catching and update snaps

* fix snaps

* add comment verification

* fix test
ziqing26 pushed a commit that referenced this issue Apr 17, 2024
ziqing26 added a commit that referenced this issue Apr 18, 2024
* Enable CI on account request form branch

* [#11878] Remove AccountRequest unique constraint (#12899)

* Remove AccountRequest unique constraint

* Remove EntityAlreadyExistsException from the throws clause

* Remove unused import of EntityAlreadyExistsException

* Fix failing checks

* Remove EntityAlreadyExistsException in dependents

* Remove assertion that is now incorrect

* Remove mysterious trailing whitespaces that appeared out of nowhere

* Remove parts in E2E test that are no longer relevant

* Remove unused import

* Improve clarity of test case

Co-authored-by: EuniceSim142 <77243938+EuniceSim142@users.noreply.github.com>

---------

Co-authored-by: EuniceSim142 <77243938+EuniceSim142@users.noreply.github.com>

* [#11878] Add status and comments to AccountRequest (#12898)

* Add AccountRequestStatus

* Add AccountRequest status attribute

* Add status to AccountRequest constructor

* Add AccountRequest comments attribute

* Add comments to AccountRequest constructor

* Wrap lines

* Remove mysterious unnecessary imports that appeared out of nowhere

* Use non-null placeholder

* Use literal placeholder

* [#11878] Add new account request alert email for admins (#12926)

* Add admin alert email

* Add email type

* Add subject

* Add email content

* Indicate that action is needed in the email subject

* [#11878] Add GetAllPendingAccountRequests API (#12927)

* add endpoint

* remove 'all' in class and method names

* fix checkstyle

* add it test

* fix checkstyle

* fix checkstyle

* fix failing test

* update endpoint url

* update it tests

* fix linting

* update param name

* update request param condition

* [#11878] Modify CreateAccountRequestAction (#12913)

* Add AccountCreateRequest instructorComments attribute

* Add new AccountRequestData attributes

* Remove check for registered instructor

* Remove sending of registration email

* Use AccountCreateRequest comments

* Change output of CreateAccountRequestAction to AccountRequestData

* Add CreateAccountRequestActionIT

* Test execute with null arguments

* Test execute with valid requests

* Test execute on invalid arguments

* Allow anybody to create an account request

* Fix architecture test

* Fix test

* Update tests to verify search indexing

* [#11878] Upgrade instructor request form UI (#12929)

* Add confirmation prompt

* Remove old form iframe

* Improve declaration view spacing

* Edit page heading phrasing for clarity

* Create request form

* Add validation messages

* Fix form validation

* Set up form submission confirmation

* Create submission acknowledgement view

* Fix URL checking regex

* Fix initial state

* Display placeholder when optional field is empty

* Fix code style

* Edit comment for clarity

* Fix institution and country combination

Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>

* Fix naming

* Remove hard line break

* Add explanatory comment for regex

* Remove newline

* Add newlines at end of file

* Clear styles file

* Re-add styles file

* Include test

* Add test cases for requestSubmissionEvent

* Improve test case readability

* Edit test case name for clarity

* Add snapshot tests

* Revert "Add snapshot tests"

This reverts commit ec7395d.

* Fix lint errors

* Rename methods to be clearer

* Disable submit button when not ready to submit

---------

Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>

* [#11878] Update Admin Home Page UI for ARF (#12933)

* create component for account request table

* cherry pick admin home page changes

* remove testing code

* fix lint and css issues

* fix admin home page snaps

* update admin home snaps

* remove edit approve and reject components

* modify css

* delete edit and reject modal components

* revert spec file changes

* integrate new types

* fix lint

* use enum for status

* fix lint

* fix css lint

* fix lint

* fix lint

* use enum and remove infinite scroll

* remove approve account request code

* remove extra div

* fix url

* modify comments

* revert extra formatting

* remove plural form and use date pipe

* fix naming

* fix spec file and update institute formatting

* fix lint

* combine institute and country columns

* [#11878] Admin Search UI Update for ARF (#12945)

* update admin search page to use acc req component

* fix selector for e2e test

* fix spec files and imports

* update e2e selector

* fix column numbers

* [#11878] Add methods to get an account request by ID (#12953)

* Add facade logic method to get an account request by ID

* Add storage method to get an account request by ID

* Add logic method to get an account request by ID

* [#11878] Add snapshot tests for instructor request form UI (#12942)

* Add snapshot tests

* Change double quotes to single quotes

* [#11878] Foundation for getting by ID in account request endpoints (#12957)

* Add account request ID query parameter

* Add account request ID to related front-end types

* Get account requests by ID in storage update method (#12955)

* Get an account request by ID in SQL injection tests (#12956)

* [#11878] Create instructor request acknowledgement email (#12944)

* Create instructor request acknowledgement email

* Add tests for acknowledgement email

* Fix test cases

* Fix comments in expected email

* Use config support email value in email template

* Fix email recipient

* Fix test expected emails

* Remove trailing space

* Use placeholder for support email

* Sanitize acknowledgement email

* Set acknowledgement email to bcc support

* [#11878] Merge master into account-request-form (#12972)

* Update chrome driver download link in e2e-testing.md (#12924)

* [#12048] Add SQL configuration into build.properties and build-dev.properties (#12917)

* Add production config

* Remove forgotten host and password

* Fix lint

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Add SQL description for postgres config (#12931)

* Add production config

* Remove forgotten host and password

* Fix lint

* Address changes, include production_user

* Linting

* [#12588] Improve test code coverage of core components - ToastComponent (#12916)

* add test cases

* add test case for isTemplate()

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>

* [#12588] Add unit tests to question edit answer form (#12935)

* add unit tests to constsum-options-question-edit-answer-form

* add unit tests to constsum-options-question-edit-answer-form

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* add delay to task queuer for indexing account request (#12936)

Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com>

* Make account req data migration script rerunnable (#12932)

* [#12048] Relax read notif verification for migration verification script (#12937)

* Fix account requests with wrong field during seed

* Relax account attributes verification

* Fix lint errors

* Fix order of account request variables

* [#12920] Create script to migrate noSQL test data to SQL schema format (#12922)

* Add classes to migrate test json data

* Add toposort  script

* Add function to remove foreign key data

* Cleanup

* WIP

* Simplify keys for students and instructors

* Fix lint issues

* Output SQL JSON in same folder as JSON

* Change output file name

* Fix bug: wrong jsonkey used

* Fix lint error

* Make section and team name unique

* Set read notification key to be unique

* Delete python file

* [#12588] Improve test code coverage of core components - ViewResultsPanelComponent (#12918)

* add test cases to ViewResultsPanelComponent

* fix lint errors

---------

Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* fix resetAccountAction (#12934)

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Migrate Feedback Rank Option E2E test (#12902)

* Initial commit

* Fix lint

* Follow convention and add test

* Change file path

* Fix requested changes

* Fixed testcases

* Fix lint

* Add deepcopy

* Fixed e2e test

---------

Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Migrate FeedbackMcqQuestionE2ETest (#12820)

* Migrate MCQ E2E

* Fix lint

* Fix lint

* Update xml

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Remove unnecessary loading of datastore entities in InstructorNotificationsPageE2ETest (#12911)

* migrate instructor notif e2e

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Migrate InstructorCourseDetailsPageE2ETest (#12908)

* Add teammates.e2e.cases.sql.InstructorCourseDetailsPageE2ETest

* Remove data properly to prevent clashes

* Add SQL data bundle

* Verify loaded details

* Use email address when getting a student row

* Check student links

* Verify the sending of invites

* Verify the reminding of all students to join

* Remove SQL data properly to prevent clashes

* Verify the downloading of the student list

* Implement helper methods for Student

* Add BaseTestCaseWithSqlDatabaseAccess::verifyAbsentInDatabase

* Add to testng-e2e-sql.xml

* Verify the deleting of students

* Verify the deleting of all the students

* Fix lint

* Remove duplicate equality check for students

* [#12588] add unit tests for question submission form (#12897)

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* Update developers.json (#12958)

* Merge pull request #12960 from TEAMMATES/master (#12961)

* [#12048] Fix account request indexing (#12967)

* Add isTransactionNeeded method to Action

* Remove delay from taskqueuer

* Change CreateAccountRequest to handle own transactions

* configure agroal connection pool (#12971)

* Fix comment style for merge

* Remove unnecessary check for account request

---------

Co-authored-by: Nada Ayesh <nayesh10@students.iugaza.edu.ps>
Co-authored-by: FergusMok <FergusMok1@gmail.com>
Co-authored-by: Maureen Chang <76696006+techMedMau@users.noreply.github.com>
Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com>
Co-authored-by: Ching Ming Yuan <cmingyuan123@gmail.com>
Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Co-authored-by: DS <yeodisheng@gmail.com>
Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>

* [#11878] Change institute length limit (#12974)

* Change institute name max length to 128

* Edit test case for new length limit

* [#11878] Update SearchAccountRequests endpoint  (#12950)

* update search document and create-core script

* update it

* modify relevant classes to use id instead of (email, institute)

* remove duplicate method

* fix it tests

* fix failing tests

* remove unnecessary comment

* [#11878] Integrate instructor request form with API (#12943)

* Integrate instructor request form FE with API

* Remove redundant statement

* Move URL regex const to backend const file

* Fix import path

* Move URL regex to FieldValidator

* Add validators to match backend fields

* Add error message box

* Change submit button display when loading

* Combine final action into subscribe

* Add max length validators for institution and country

* Fix lint errors

* Add test cases to test submission

* Add specific error messages for form validation

* Remove home page URL field

* Fix lint errors

* Remove url regex from test

* Update snap

* Clean up test code

* Remove comment about home page URL

* Change canSubmit check to getter

* Fix form submit button not re-enabling on error

* Add name pattern validator to front-end

* Fix snapshot

* [#11878] Create Update Account Request Action (#12982)

* create update action and IT

* update javadocs

* update tests

* add more tests

* simplify logic

* remove unused string

* fix test

* allow null comments

* add more tests

* use EntityNotFoundException

* cleanup after create account requests test

* remove unncessary check

* [#11878] Fix Account Request Update Search Indexing (#12984)

* update account request indexing

* add methods to test access control

* refactoring for transactions

* [#11878] Add Edit and Approve Account Requests functionality (#12975)

* add edit and approve functionality

* remove rejection code

* fix snap

* integrate endpoint

* disable approve button for approved requests

* use comments instead of comment

* use searchString instead of searchQuery

* fix snap

* [#11878] Add AccountRequest Rejection email generator. (#12987)

* add rejection-email template and email generator method

* add javadoc for email generator method

* add test

* fix test method names

* fix test method name 2

* fix lint

* add bcc for rejection email

* [#11878] Create reject account request endpoint (#12985)

* Create account request rejection endpoint

* Add validation

* Add check for already rejected request when sending email

* Add integration test cases

* Set request method to post

* Fix lint errors

* Update tests list

* Update validation check

* Add test for validation

* Fix lint errors

* Fix validation comparison

* Fix error message test

* Add email sending

* Update test cases

* Refactor reason check code for clarity

* Remove unused modal (#12998)

* use transactions for reject account request action (#13001)

* [#11878] Create Rejection Modal for Account Requests (#12989)

* Create rejection modal

* fix lint and tests

* fix placeholders and lint

* remove title

* integrate api

* check undefined title and body

* fix trailing white spaces

* fix whitespace

* change error message

* re-add account request table on home page

* replace support email

* [#11878] Update DeleteAccountRequest to reference by ID (#12997)

* Update to delete by id

* fix lint

* fix lint

* fix frontend lint

* [#11878] Update ResetAccountRequest to reference by ID (#13002)

* Update reset to reference by id

* fix comments

* [#11878] Add Error Message for Approving Existing Account (#13004)

* add error message for duplicate account request

* add tests

* [#11878] Get account request by uuid (#13007)

* change GetAccountRequestAction to get by id

* fix tests

* remove unncessary todo

* [#11878] Handle Duplicate Approved Account Requests (#13009)

* [#11878] Merge master into feature (#13011)

* Update chrome driver download link in e2e-testing.md (#12924)

* [#12048] Add SQL configuration into build.properties and build-dev.properties (#12917)

* Add production config

* Remove forgotten host and password

* Fix lint

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Add SQL description for postgres config (#12931)

* Add production config

* Remove forgotten host and password

* Fix lint

* Address changes, include production_user

* Linting

* [#12588] Improve test code coverage of core components - ToastComponent (#12916)

* add test cases

* add test case for isTemplate()

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>

* [#12588] Add unit tests to question edit answer form (#12935)

* add unit tests to constsum-options-question-edit-answer-form

* add unit tests to constsum-options-question-edit-answer-form

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* add delay to task queuer for indexing account request (#12936)

Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com>

* Make account req data migration script rerunnable (#12932)

* [#12048] Relax read notif verification for migration verification script (#12937)

* Fix account requests with wrong field during seed

* Relax account attributes verification

* Fix lint errors

* Fix order of account request variables

* [#12920] Create script to migrate noSQL test data to SQL schema format (#12922)

* Add classes to migrate test json data

* Add toposort  script

* Add function to remove foreign key data

* Cleanup

* WIP

* Simplify keys for students and instructors

* Fix lint issues

* Output SQL JSON in same folder as JSON

* Change output file name

* Fix bug: wrong jsonkey used

* Fix lint error

* Make section and team name unique

* Set read notification key to be unique

* Delete python file

* [#12588] Improve test code coverage of core components - ViewResultsPanelComponent (#12918)

* add test cases to ViewResultsPanelComponent

* fix lint errors

---------

Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* fix resetAccountAction (#12934)

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Migrate Feedback Rank Option E2E test (#12902)

* Initial commit

* Fix lint

* Follow convention and add test

* Change file path

* Fix requested changes

* Fixed testcases

* Fix lint

* Add deepcopy

* Fixed e2e test

---------

Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Migrate FeedbackMcqQuestionE2ETest (#12820)

* Migrate MCQ E2E

* Fix lint

* Fix lint

* Update xml

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Remove unnecessary loading of datastore entities in InstructorNotificationsPageE2ETest (#12911)

* migrate instructor notif e2e

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12048] Migrate InstructorCourseDetailsPageE2ETest (#12908)

* Add teammates.e2e.cases.sql.InstructorCourseDetailsPageE2ETest

* Remove data properly to prevent clashes

* Add SQL data bundle

* Verify loaded details

* Use email address when getting a student row

* Check student links

* Verify the sending of invites

* Verify the reminding of all students to join

* Remove SQL data properly to prevent clashes

* Verify the downloading of the student list

* Implement helper methods for Student

* Add BaseTestCaseWithSqlDatabaseAccess::verifyAbsentInDatabase

* Add to testng-e2e-sql.xml

* Verify the deleting of students

* Verify the deleting of all the students

* Fix lint

* Remove duplicate equality check for students

* [#12588] add unit tests for question submission form (#12897)

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* Update developers.json (#12958)

* Merge pull request #12960 from TEAMMATES/master (#12961)

* [#12048] Fix account request indexing (#12967)

* Add isTransactionNeeded method to Action

* Remove delay from taskqueuer

* Change CreateAccountRequest to handle own transactions

* configure agroal connection pool (#12971)

* [#12048] Configure connection pool using hikari (#12978)

* Configure hikari

* Remove spacing

* Lint

* [#12048] Update liquibase configuration (#12930)

* Update gradle config

* Update liquibase config for v9

* Turn off table generate for prod

* Update of changelog file

* Add configuration for generating changelog

* Add schema migration docs

---------

Co-authored-by: FergusMok <FergusMok1@gmail.com>

* [#12048] Migrate AccountRequestsLogicTest (#12780)

* Migrate test cases for AccountRequestsLogic

* Remove test case

* Split test cases

* [#12048] Migrate AdminSearchPageE2ETest SQL (#12811)

* test e2e changes

* fix: reduce e2e test json file size

* fix student key

* fix course key

* fix instructor keys

* fix filepath

* fix e2e test

* remove extra data from bundle

* Add correct removal logic to avoid constraint violation

* Fix e2e tests and lint

fix reset google id test

fix e2e tests

fix e2e tests

fix tests

remove double click

fix unknown symbol

add toast check

change toast verification message

remove toast check

* fix: add null check

* move admin search page e2e test to sql cases

* Rename AdminSearchPageE2ETest_SQLEntities.json to AdminSearchPageE2ETest_SqlEntities.json

* fix failing test

* fix: remove extra null check

* fix: add test to e2e sql xml file

* fix function call

* remove unnecessary changes

* create new file for sql entities

* revert unnecessary changes

* remove trailing whitespace

* add teardown for account requests

---------

Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>

* [#12995] Create documentation for unit tests (#12996)

* Create documentation for unit tests

* Update docs/unit-testing.md

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* Update docs/unit-testing.md

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Remove feedbackSession attributes @fetch annotation (#12992)

* Remove feedbackSession @fetch annotation

* [#12048] create skeleton for sql LNP tests (#12994)

* create skelton for sql LNP tests

* allow lnp test to access sql storage and ensure sql lnp tests are independant of each other

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>

* [#12048] Migrate FeedbackNumScaleQuestionE2ETest (#12940)

* Migrate num scale e2e

* Fix team id

* Fix bugs

* Add v9.0.0 tag to liquibase changelog (#13005)

* sort courses by id before comparison (#13003)

Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>

---------

Co-authored-by: Nada Ayesh <nayesh10@students.iugaza.edu.ps>
Co-authored-by: FergusMok <FergusMok1@gmail.com>
Co-authored-by: Maureen Chang <76696006+techMedMau@users.noreply.github.com>
Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com>
Co-authored-by: Ching Ming Yuan <cmingyuan123@gmail.com>
Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Co-authored-by: DS <yeodisheng@gmail.com>
Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>
Co-authored-by: Xenos F <git@xenosfio.com>
Co-authored-by: domoberzin <74132255+domoberzin@users.noreply.github.com>
Co-authored-by: Marques Tye Jia Jun <97437396+marquestye@users.noreply.github.com>

* [#11878] Add tests for Account Request Table (#12977)

* add component tests for account request table

* modify tests

* remove old tests

* remove comment

* remove unnecessary code

* add tests

* update disabled criteria

* remove extra builders and update snaps

* [#11878] Reference account requests by ID in tests (#13017)

* Reference by ID in GetCourseJoinStatusActionIT

* Reference by ID in AccountRequestsDbIT

* Reference by ID in AccountRequestsLogicIT

* Reference by ID in CreateAccountActionIT

* Reference by ID in BaseTestCaseWithSqlDatabaseAccess

* Remove now irrelevant reference by email address and institute

* [#11878] Fix Approval Email Bug (#13027)

* [#11878] Fix reject email content (#13029)

* [#11878] Add Toasts (#13028)

* add toasts

* lint

* [#11878] Remove mention of home page URL from confirmation email (#13030)

* fix highlighting and null statuses (#13031)

* [#11878] Fix Overlapping Tooltip (#13026)

* fix tooltip

* re-add removed lines

* update snap

* update snaps

* lint

* suppress warning

* lint

* [#11878] Check if account request is not created by admin before sending email (#13032)

* Add check if admin when creating account request

* Trim down test case

* [#11878] Add sort by created_at for getAllPendingRequests (#13038)

* feat: add order-by

* fix: fix failing test case

* fix: add missing created-at field

* fix: test case comment

* [#11878] Request Page E2E (#13015)

* Add E2E skeleton

* Fix test and lint

* Add verifyEmailSent

* fix fe tests

* Remove method from logic and db (#13044)

* [#11878] Migrate AccountRequestsLogic unit tests (#13043)

* Add get typical account request method

* Migrate AccountRequestsLogicTest

* Remove test for get by email address and institute

---------

Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>

* [#11878] Add Admin E2E Tests (#13020)

* add admin e2e tests

* remove exception catching and update snaps

* fix snaps

* add comment verification

* fix test

* [#11878] Add SQLI tests (#13047)

* Revert "Enable CI on account request form branch" (#13049)

This reverts commit 186a97a.

---------

Co-authored-by: Jay Ting <65202977+jayasting98@users.noreply.github.com>
Co-authored-by: EuniceSim142 <77243938+EuniceSim142@users.noreply.github.com>
Co-authored-by: Xenos F <git@xenosfio.com>
Co-authored-by: domoberzin <74132255+domoberzin@users.noreply.github.com>
Co-authored-by: Nada Ayesh <nayesh10@students.iugaza.edu.ps>
Co-authored-by: FergusMok <FergusMok1@gmail.com>
Co-authored-by: Maureen Chang <76696006+techMedMau@users.noreply.github.com>
Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com>
Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com>
Co-authored-by: Ching Ming Yuan <cmingyuan123@gmail.com>
Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Co-authored-by: DS <yeodisheng@gmail.com>
Co-authored-by: Marques Tye Jia Jun <97437396+marquestye@users.noreply.github.com>
ziqing26 added a commit that referenced this issue Apr 18, 2024
ziqing26 pushed a commit that referenced this issue Apr 22, 2024
* add script

* order commit

* update logic

* convert to batch

* remove print
jayasting98 added a commit that referenced this issue Apr 23, 2024
* Add captcha to ARF

* Update front-end tests

* Fix lint errors

* Change captcha to uppercase in error text

* Return captcha response when the getter is called

---------

Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Epic Feature/task that is worth many smaller sub-features/sub-tasks committers only Difficult; better left for committers or more senior developers enhancement New feature or request
Projects
None yet
3 participants