-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add support for limit submit and unique field in store data option #58
Conversation
@@ -85,7 +85,7 @@ def add(self, data): | |||
record = Record() | |||
fields_labels = {} | |||
fields_order = [] | |||
for field_data in data: | |||
for field_data in data["form_data"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ma, c'era un bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no no, è solo perché ho aggiunto più campi al json con i dati
|
||
if not unique: | ||
raise ValueError(f" {', '.join([x[1] for x in keys])}") | ||
|
||
return self.soup.add(record) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunque, se capisco bene, guardi i campi che arrivano nel form e cerchi nei dati salvati finchè non vedi che c'è un unicità.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esatto, avrei voluto usare modi più veloci, ma dato che l'unicità dei campi può cambiare tramite il form va tutto fatto al volo
records = self.soup.data.values() | ||
|
||
return len(records) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, questo ti ritorna il numero di entry. non capisco come lo colleghi al form... mi pare che così cerchi in tutto il soup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onestamente avevo anche io quel dubbio, ma mi sono basato sulla search per farlo, e la search funziona
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eikichi18 IMHO this is a bug. Try adding 2 forms, each with limit 2, add one record to each, try adding a second record. Does this work?
if self.form_block.get("limit", None) is not None: | ||
limit = int(self.form_block["limit"]) | ||
if limit > -1: | ||
custom_colums.append("waiting_list") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quindi, per fare il csv, se si usa anche il limite (> -1) aggiungi la colonna. ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esatto, altrimenti la colonna non la metto
@@ -83,8 +88,12 @@ def get_data(self): | |||
# add fixed columns values | |||
value = item.attrs.get(k, None) | |||
data[k] = json_compatible(value) | |||
if "waiting_list" in custom_colums: | |||
data.update({"waiting_list": not (index < limit)}) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, quindi decidi di gestirla con waiting => true /false.
va bene. non abbiamo indicazioni in merito. però è per non informatici. metterei 'Sì' / 'No'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fatto
context=self.request, | ||
) | ||
self.request.response.setStatus(500) | ||
return dict(type="InternalServerError", message=message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, abbiamo visto che è l'unico problema che può capitare? può anche essere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prima la gestione dell'errore non era presente, per cui ho gestito l'eccezione di cui ho messo il raise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Che ValueError equivale sempre a "valore duplicato" a me pare una supposizione azzardata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concordo con te, mi sembrava l'errore python più vicino al nostro problema, posso comunque creare una classe eccezione ad hoc per quel problema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fatto
|
||
def count_data(self): | ||
store = getMultiAdapter((self.context, self.request), IFormDataStore) | ||
return store.count() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quindi, aggiorni passando il valore della waiting list al momento della scrittura. ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non so se sia il caso di mettere accrocchi che controllino l'esecuzione l'esecuzione di quel codice con dei lock... della serie, se due thead stanno inserendo l'ultima persona prima della waiting list? mi pare che non capiti nulla: ad ogni modo, uno dei due arriva prima dell'altro...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dipende quando è critica la faccenda, per come la vedo io è comunque una domanda per qualcosa, quindi tutti i submit verranno poi revisionati da qualcuno in questo caso. Per questo motivo non la vedevo una cosa super critica
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunque, mi pare tutto abbastanza bene. mi pare nel ticket si parlasse di avvisare anche nella mail che viene inviata. ricordo male @eikichi18 ?
sì, stavo rivedendo adesso e lo dice, ma ho un pò di dubbi, intanto il limite può cambiare quindi le informazioni che arrivano per email possono variare in qualsiasi momento. Altra cosa l'email di base non da informazioni particolari sulla cosa per cui tu hai mandato una richiesta, fa solo il riepilogo dei dati che hai mandato. Ho preferito non alterarne il significato |
This is a community package, so please use english in discussions ;) Another point is: is it possible to move these features into a separate plugin? I don't want to add too many extra features into a form product (if i understand correctly you want to use it as a registration tool) |
It's the right point to move the logics in another package, as we want to do here for example: #56 |
Added support for field limit that limits form submission and the exceeding ones are added to a waiting list.
also added support to make one or more fields unique