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

Allow dynamic instance method calls on FactoryBot objects in Cypress #696

Merged
merged 16 commits into from
Sep 25, 2024

Conversation

Splines
Copy link
Member

@Splines Splines commented Sep 24, 2024

Cypress API changes

A first API for this was proposed in #671. This is a redesign of that API to make things more scalable and easier to use from within Cypress tests. With the changes in this PR, we are able to write something like this in Cypress tests:

FactoryBot.create("lecture", { teacher_id: this.teacher.id }).as("lecture");

cy.then(() => {
  this.lecture.call.long_title().then((res) => {
    cy.log(res);
  });
});

Call .call on any instance you've created with FactoryBot. On the resulting object, you can call any method, e.g. here long_title(). This will translate to a method call on the respective instance in the backend.
You can even pass (multiple) arguments to this method, but of course this only works for primitive data types such as strings and integers and not when the function excepts a user object or other complex Rails objects.

For all supported method calls, see the respective tests under cypress/e2e/meta/factory_bot_spec.cy.js.

My plan is to merge this PR into dev, then remove the first proposal API from #671, then merge dev into #671 and use the new API there for the tests. This also makes PR #671 smaller.

Off-topic

  • In Cypress, we can now create multiple users of the same role, e.g. multiple generic users. This is achieved by adding a random hash to their email such that there is no duplicate key.
  • In the Cypress JavaScript files, we don't store the default Cypress password anymore. Instead, it is returned by the backend via the controllers/cypress/user_creator_controller.rb.

For reviewers

  • It's not worth to browse through my commits here as it's only in the last ones that everything works. Before that, I experimented a lot.
  • Convention in JS: methods that should be considered private start with a #.
  • There is a lot of "JavaScript magic" going on here. In order to understand the mechanism with a simpler example, read through my question here. The answers to this questions were the reason I introduced the .call syntax instead of allowing dynamic method calls directly on the resulting object. The function defined in the StackOverflow question is very similar to the final one in this PR, namely #allowDynamicMethods() of the factorybot.js. Setting the .call property is done in #defineCallProperty().
  • In the code here, the complexity increases because of how Cypress handles things in the .as() and .then() methods with its own wrapping mechanisms. This is handled in #createProxy().
  • To see how multiple arguments could be handled, see this commit. You might want to check out one commit before that to test it locally, i.e. git checkout f20d8120dc.

Out of scope

  • This PR also refactors the create() method of the Rails FactoriesController according to some changes already done in Vouchers for user promotion - Part 2: Redemption #671, e.g. we introduce the method validate_factory_name.
  • Do we also want to keep the transform_hash method? Could it still be useful? If yes, could you please add a docstring explaining what it does, e.g. input/output as I'm not too sure 😅

@Splines Splines added the tests Unit tests, integration tests etc. label Sep 24, 2024
@Splines Splines self-assigned this Sep 24, 2024
Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.45%. Comparing base (49cf16a) to head (6ed2190).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #696   +/-   ##
=======================================
  Coverage   54.45%   54.45%           
=======================================
  Files         160      160           
  Lines        6751     6752    +1     
=======================================
+ Hits         3676     3677    +1     
  Misses       3075     3075           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

app/controllers/cypress/factories_controller.rb Outdated Show resolved Hide resolved
app/controllers/cypress/factories_controller.rb Outdated Show resolved Hide resolved
spec/cypress/support/factorybot.js Show resolved Hide resolved
@Splines
Copy link
Member Author

Splines commented Sep 25, 2024

@fosterfarrell9 Thanks for your review, I've incorporated all your suggestions. Please take note of the "off-topic" section I've introduced in the PR comment.

@Splines Splines changed the title Allow dynamic method calls for FactoryBot objects in Cypress Allow dynamic instance method calls on FactoryBot objects in Cypress Sep 25, 2024
This failed due to random hash added to the mail address
in the last commit.
Copy link
Collaborator

@fosterfarrell9 fosterfarrell9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Splines Splines merged commit 76e3329 into dev Sep 25, 2024
5 checks passed
@Splines Splines deleted the feature/cypress-rails-dynamic branch September 25, 2024 20:56
Splines added a commit that referenced this pull request Sep 25, 2024
(in order to call instance methods, see #696
fosterfarrell9 added a commit that referenced this pull request Nov 10, 2024
* Initialize voucher model and add some unit tests

* Make ensure_no_other_active_voucher validation into a callback

* Add throw :abort in order to halt execution

* Replace Time.now by Time.zone.now

* Set up basic functionality for display of vouchers for tutors

* Create separate file for copy and paste button code and decaffeinate

* Set up destruction of tutor vouchers

* Rename view file as it containes embedded ruby

* Add create action for vouchers and corresponding views

* Adapt views and controller for adding and removing of vouchers of different type

* Put duplicate lines into a separate method

* Set up redeeming of vouchers

* fix typo

* remove obsolete methods

* Avoid use of Time.now

* Refactor active_vouhcer_of_sort method

* remove unused expired? method

* Remove duplicate code

* remove unused variable

* Add controller spec for vouchers

* Rewrite controller spec for vouchers

* remove obsolete comment

* Set up redemption of tutor vouchers

* Remove user lookup for tutor positions

* Set up redemption of editor vouchers

* Add Contract Model and setup selection of tutors and editors

* Set up notifications for redemption of tutor vouchers

* Invalidate vouchers instead of destroying them

* Add redemption and claim models and rewrite workflow accordingly

* Restore to previous state

* Fix typo

* Implement Rubocop suggestion

* Replace voucher_hash by secure_hash

* Replace redeem_voucher_params by check_voucher_params

* Add cancel action to vouchers controller

* Differentiate between different cases of tutor voucher redemption

* Differentiate between different cases of editor voucher redemption

* Take first steps in setting up redemption of teacher vouchers

* Add notifications for redemption of teacher vouchers

* Add notification mails for teacher change by voucher redemption

* Invalidate teacher vouchers after first use

* Add vouchers for seminar speakers

* Add helpdesk for speaker vouchers

* Set up redemption of speaker vouchers

* Add notifications for redemption of speaker vouchers

* Restrict access user select

* Remove unused parameter

* Introduce VoucherProcessor service object

* Use common partial for redemption of tutor and speeaker vouchers

* Rename sort attribute of vouchers to role

* Rename remaining occurences of sort attribute of voucher to role

* Add unit tests for VoucherProcessor

* Add .uniq to method

* Remove teacher selection for non-admins

* Remove obsolete reference to non-existent model

* Remove permissions from non-admins to change teachers

* Add cypress data attributes

* Add first cypress tests

* Add more cypress tests

* Add first cypress tests for voucher redemption

* Init future possibility to check clipboard content

* Add missing teacher field for non-admins

* Add cypress tests for redemption form

* Add cypress command .tomselect to select in TomSelect forms

* Add test for redemption form using the new .tomselect command

* Remove unnecessary call of trait

* Use NO_SEMINAR_ROLES constant

* Clean up some experiments

* Remove .tomselect Command for cypress

* Enlarge test user json by some data

* Add logout command and expect correct statuses for login and logout requests

* Add more cypress tests for redemption of tutor vouchers

* Parse hashes with integer keys into arrays of strings

* Add tests for tutor voucher redemption in the case that all tutorials are already taken

* Add possibility to pass instance methods and refactor

* Update documentation

* Add tests for voucher redemption that check notifications

* Add missing speaker voucher case

* Add missing locales

* Add cypress tests fo editor vouchers, teacher vouchers and speaker vouchers

* Remove out of scope test

* Refactor helper methods

* Extract helper functions to separate file

* Refactor tests

* Extract more methods into helper

* Rename files according to conventions

* Add controller specs for changed lecture update policy

* Refactor tests by extracting methods

* Don't define `redeem` method twice

Apparently I missed that one during the merge.

* Pluralize voucher redemption spec filename

* Delete duplicate migration & update new migration dates/schema

* Move cypress helpers to e2e folder

`support` folder should be used for general test-stuff, i.e. not-related
to individual, specific tests.

* Improve naming for voucher finding method

also moved one private function around

* Inline JSON object in user creator controller

* Use symbolic keys upon voucher type case statement

* Fix "Redeem voucher" spelling mistake

* Use fixed width for voucher redemption selectize field

* Rename view file to `voucher_redemption_form`

* Fix indentation in view file (tutor voucher)

* Replace cypress `describe` by `context`

* Remove reviewer-TODO comments (selectize tomselect)

* Reset Cypress-related code to 49cf16a

Command used:
git checkout 49cf16a -- ./.vscode/settings.json ./app/controllers/cypress/factories_controller.rb ./app/controllers/cypress/user_creator_controller.rb ./spec/cypress/support/factorybot.js

* Use new .call syntax in cypress tests

(in order to call instance methods, see #696

* Explicitly set `name_in_tutorials` for users

* Remove constraint that user id must be a number

This is such that UUIDs also work.

* Init support for entity-relationship-diagram creation

* Add `has_many :notifications` for better ERD visibility

* Remove usage of delegate methods from redemption

* Don't hardcode source_type as string

* Refactor voucher_processor into model concern

According to concepts discussed in #694.
Also auto-load subdirectories in models folder
and set Current.user for usage in models.

* Add missing notification and subscribe lecture steps

* Move vouchers down in UI (lecture edit page)

* Turn off autocomplete for redeem voucher text field

* Strip secure hash to avoid copy-pasting issues with voucher

* Improve voucher-model docstrings

* Fix wrong permit syntax for arrays

* Move Claim model to voucher folder

* Adapt Redeemer specs to new architecture

Also rename Redeemable to Redeemer to better reflect what it's doing.

* Use `pluck` instead of `select` for better performance

Also use .uniq just to be sure.

* Test timestamp in invalidate! voucher spec

* Remove unused routes

* Fix missing CSpell configuration

* Improve tutor voucher redemption messages

also remove unused locale keys

* Improve speaker voucher redemption messages

* Replace "Voucher" by "Gutschein" in German texts

* Improve teacher voucher redemption messages

* Improve editor voucher redemption messages

* Remove duplicate `no_active_voucher` i18n key

* Add help texts to voucher creation

* Also eager_load additional modules

This is to ensure that `Rails.application.eager_load!` in the rails_erd
gem respects all our modules.

See this line:
https://github.com/voormedia/rails-erd/blob/7c66258b6818c47b4d878c2ad7ff6decebdf834a/lib/rails_erd/tasks.rake#L45

* Outsource x_by_redemption methods from lecture to redemption

* Fix wrong i18n controller error

The code always errored since the JSON objects are passed as strings
from the frontend.

* Fix cypress test (due to renaming of i18n keys / html)

* Cypress test that whitespaces in voucher hash work

* Verify voucher is invalid after user became teacher

* Add user deletion cypress spec (for tutor)

* Add word to cspell list

* Replace "display" by "show"

For me, "display" is closer to the frontend, while this is a backend test,
that's why I renamed it. Not really that important in the end ;)

* Fix two spelling mistakes (out of scope)

* Move lecture notifications to separate file

* Replace Notifier concern by LectureNotifier module

* Remove unused set_recipients method for "new editor mail"

* Move email templates to right folder

* Correct sender_nad_locale setting

* Introduce enqueue_mail_with_params matcher & test for editor

* Add spec for Current user model

* Inline sender_and_locale

* Try to test email sending for editor voucher (fails)

* Mock Current model in RSpec tests

* Rework sender and locale setting

* Fix mail sending test for editor

* Test mail body (editor)

* Set locale in broader scope test

* Fix comment

* Test previous and new teacher mail

* Use custom html body matcher (ignore \r\n)

* Improve wording in html body failure message

* Test mails for co-speakers

* Ensure user that is now speaker doesn't receive a mail

* Use just one file for mail matchers

* Rename matcher to enqueue_mail_including_params

* Outsource from notification mailer assertion

* Refactor send mail to co-speaker test

* Remove unwanted test

I accidentally added this even though it's not the
wanted behavior.

* Add tests for teacher selection dropdown

* Write test for editor dropdown selection

* Allow admin to select any user in lecture editor select

* Remove `only` from cypress test

* Add more words to spell checker

* Add tests for tutor selection dropdown

* Add back ability for admin to select any user as tutor

* Allow admin to choose arbitrary users as speakers & test

* Remove unnecessary display: none

* Improve if statement in voucher redemption

* Remove another unnecessary "be.visible" statement

* Allow admins to select arbitrary users in existing talks & test

* Rename lecture spec file

* Refactor lecture people select spec (extract methods)

* Reuse speaker_select helper

* Outsource to new teacher_select helper

* Outsource to new editors_select helper

* Remove with_seminar trait

(since we use the association field now, introduced
a few commits beforehand)

* Make cypress input selecting more reliable

* Remove unnecessary div wrap

* Avoid flaky test by intercepting user fill request

* Fix cypress not typing "cy" in input box

* Remove current_user= test as method was removed

* Intercept /talks/new route to avoid flaky test

This is due to the form not being completely loaded while we
already go on.

* Add cy.wait as last resort

* Visit #people page directly

* Remove unwanted `only` in Cypress test

---------

Co-authored-by: Splines <dominic-plein@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Unit tests, integration tests etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants